[libvirt] [PATCH 2/2] qemu: Regenerate VNC socket paths
Pavel Hrdina
phrdina at redhat.com
Wed Apr 27 09:36:19 UTC 2016
On Wed, Apr 27, 2016 at 09:15:16AM +0200, Martin Kletzander wrote:
> On Wed, Apr 27, 2016 at 06:24:43AM +0200, Pavel Hrdina wrote:
> >On Tue, Apr 26, 2016 at 04:34:12PM +0200, Martin Kletzander wrote:
> >> Similarly to what commit 714080791778 did with some internal paths,
> >> clear vnc socket paths that were generated by us. Having such path in
> >> the definition can cause trouble when restoring the domain. The path is
> >> generated to the per-domain directory that contains the domain ID.
> >> However, that ID will be different upon restoration, so qemu won't be
> >> able to create that socket because the directory will not be prepared.
> >>
> >> Best viewed with '-C'.
> >>
> >> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1326270
> >>
> >> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> >> ---
> >> src/qemu/qemu_domain.c | 21 +++++++++++-
> >> .../qemuxml2argv-graphics-vnc-autosocket.args | 22 ++++++++++++
> >> .../qemuxml2argv-graphics-vnc-autosocket.xml | 34 +++++++++++++++++++
> >> .../qemuxml2xmlout-graphics-vnc-autosocket.xml | 39 ++++++++++++++++++++++
> >> tests/qemuxml2xmltest.c | 7 ++++
> >> 5 files changed, 122 insertions(+), 1 deletion(-)
> >> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.args
> >> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-autosocket.xml
> >> create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-vnc-autosocket.xml
> >>
> >> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> >> index a2f981043915..d6f704d6f91b 100644
> >> --- a/src/qemu/qemu_domain.c
> >> +++ b/src/qemu/qemu_domain.c
> >> @@ -1659,10 +1659,26 @@ qemuCanonicalizeMachine(virDomainDefPtr def, virQEMUCapsPtr qemuCaps)
> >> }
> >>
> >>
> >> +static void
> >> +qemuDomainCleanupInternalPaths(virDomainDefPtr def, virQEMUDriverConfigPtr cfg)
> >> +{
> >> + size_t i = 0;
> >> +
> >> + for (i = 0; i < def->ngraphics; ++i) {
> >> + virDomainGraphicsDefPtr graphics = def->graphics[i];
> >> +
> >> + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
> >> + graphics->data.vnc.socket &&
> >> + STRPREFIX(graphics->data.vnc.socket, cfg->libDir))
> >> + VIR_FREE(graphics->data.vnc.socket);
> >> + }
> >
> >I would also move the channel socket cleanup code here so we don't have two
> >places doing the same thing.
> >
>
> That would introduce another loop over devices and one of the main
> points of DevicePostParse is not having to do that.
Yes, I know and still I thing that we should have only one place to cleanup
those socket paths so it could be reused for the migration cleanup.
>
> >> +}
> >> +
> >> +
> >> static int
> >> qemuDomainDefPostParse(virDomainDefPtr def,
> >> virCapsPtr caps,
> >> - unsigned int parseFlags ATTRIBUTE_UNUSED,
> >> + unsigned int parseFlags,
> >> void *opaque)
> >> {
> >> virQEMUDriverPtr driver = opaque;
> >
> >This isn't enough to fix the managedsave/migration. The root cause is that we
> >generate wrong migratable XML. This cleanups the definition before migratable
> >XML is created:
> >
>
> It is enough, but I agree that's not the root cause. Even though this
> will fix it when migrating to new libvirt from whatever version, we want
> to be able to do migrate from version with this patch in, so I'll amend that.
>
> >diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> >index 2e409d4..dc0b035 100644
> >--- a/src/qemu/qemu_domain.c
> >+++ b/src/qemu/qemu_domain.c
> >@@ -2560,6 +2560,7 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver,
> > virCPUDefPtr cpu = NULL;
> > virCPUDefPtr def_cpu = def->cpu;
> > virDomainControllerDefPtr *controllers = NULL;
> >+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> > int ncontrollers = 0;
> > virCapsPtr caps = NULL;
> >
> >@@ -2646,7 +2647,7 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver,
> > }
> > }
> >
> >-
> >+ qemuDomainCleanupInternalPaths(def, cfg);
>
> However, we cannot do that because we would erase that from the live
> XML. What we need to do instead is make sure we don't format that path
> if flags & VIR_DOMAIN_FORMAT_XML_MIGRATABLE is true (or whatever the
> flag name is).
>
> I'll send a v2 with that
If you look closely at the location where I've put that call, you'll see, that
it's in qemuDomainDefFormatBuf() in "if ((flags & VIR_DOMAIN_XML_MIGRATABLE))"
condition which is exactly what we want.
Pavel
>
> > }
> >
> > ret = virDomainDefFormatInternal(def, driver->caps,
> >@@ -2662,6 +2663,7 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver,
> > def->ncontrollers = ncontrollers;
> > }
> > virObjectUnref(caps);
> >+ virObjectUnref(cfg);
> > return ret;
> > }
More information about the libvir-list
mailing list