[libvirt] [PATCH 2/2] qemu: Regenerate VNC socket paths

[Martin Kletzander]

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.

>> +}
>> +
>> +
>>  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

>     }
>
>     ret = virDomainDefFormatInternal(def, driver->caps,
>@@ -2662,6 +2663,7 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver,
>         def->ncontrollers = ncontrollers;
>     }
>     virObjectUnref(caps);
>+    virObjectUnref(cfg);
>     return ret;
> }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160427/0709e940/attachment-0001.sig>