[libvirt PATCH 5/6] Make PATHs unique for a VM object instance

Martin Kletzander mkletzan at redhat.com
Tue Feb 11 09:05:53 UTC 2020


On Wed, Feb 05, 2020 at 05:32:50PM +0000, Daniel P. Berrangé wrote:
>On Mon, Feb 03, 2020 at 12:43:32PM +0000, Daniel P. Berrangé wrote:
>> From: Shaju Abraham <shaju.abraham at nutanix.com>
>>
>> There are various config paths that a VM uses. The monitor paths and
>> other lib paths are examples. These paths are tied to the VM name or
>> UUID. The local migration breaks the assumption that there will be only
>> one VM with a unique UUID and name. During local migrations there can be
>> multiple VMs with same name and UUID in the same host. Append the
>> domain-id field to the path so that there is no duplication of path
>> names.
>
>This is the really critical problem with localhost migration.
>
>Appending the domain-id looks "simple" but this is a significant
>behavioural / functional change for applications, and I don't think
>it can fully solve the problem either.
>
>This is changing thue paths used in various places where libvirt
>internally generates unique paths (eg QMP socket, huge page or
>file based memory paths, and defaults used for auto-filling
>device paths (<channel> if not specified).
>
>Some of these paths are functionally important to external
>applications and cannot be changed in this way. eg stuff
>integrating with QEMU can be expecting certain memory backing
>file paths, or certain <channel> paths & is liable to break
>if we change the naming convention.
>
>For sake of argument, lets assume we can changing the naming
>convention without breaking anything...
>

This was already done in (I would say) most places as they use
virDomainDefGetShortName() to get a short, unique name of a directory -- it uses
the domain ID as a prefix.

>This only applies to paths libvirt generates at VM startup.
>
>There are plenty of configuration elements in the guest XML
>that are end user / mgmt app defined, and reference paths in
>the host OS.
>
>For example <graphics>, <serial>, <console>, <channel>,
>all support UNIX domain sockets and TCP sockets. A UNIX
>domain socket cannot be listened on by multiple VMs
>at once. If the UNIX socket is in client mode, we cannot
>assume the thing QEMU is connecting to allows multiple
>concurrent connections. eg 2 QEMU's could have their
><serial> connected together over a UNIX socket pair.
>Similarly if automatic TCP port assignment is not used
>we cannot have multiple QEMU's listening on the same
>host.
>
>One answer is to say that localhost migration is just
>not supported for such VMs, but I don't find that very
>convincing because the UNIX domain socket configs
>affected are in common use.
>

I would be okay with saying that these either need to be changed in a provided
destination XML or the migration will probably break.  I do not think it is
unreasonable to say that if users are trying to shoot themselves, we should not
spend a ridiculous time just so we can prevent that.  Otherwise we will get to
the same point as we are now -- there might be a case where a local migration
would work, but users cannot execute it even if they were very cautious and went
through all the things that can prevent it from the technical point of view,
libvirt will still disallow that.

Adding at least partial support where we could say we rely on QEMU failing
reasonably would allow couple of mgmt apps to do more than they can do now.  And
they might have taken care of the path collisions (e.g. when running libvirtd in
containers or so).

>If localhost migration is only usable in a small subset
>scenarios, I'm not convinced it is worth the support
>burden. Rarely used & tested features are liable to
>bitrot, and migration is already a serious point of
>instability in QEMU in general.
>
>> Signed-off-by: Shaju Abraham <shaju.abraham at nutanix.com>
>> ---
>>  src/qemu/qemu_conf.c   |  4 ++--
>>  src/qemu/qemu_domain.c | 16 ++++++++++------
>>  2 files changed, 12 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
>> index 00801ef01b..6769736d58 100644
>> --- a/src/qemu/qemu_conf.c
>> +++ b/src/qemu/qemu_conf.c
>> @@ -1894,7 +1894,7 @@ qemuGetDomainHugepagePath(const virDomainDef *def,
>>      char *ret = NULL;
>>
>>      if (base && domPath)
>> -        ret = g_strdup_printf("%s/%s", base, domPath);
>> +        ret = g_strdup_printf("%s/%s-%d", base, domPath, def->id);
>>      return ret;
>>  }
>>
>> @@ -1962,7 +1962,7 @@ qemuGetMemoryBackingDomainPath(const virDomainDef *def,
>>          return -1;
>>
>>      qemuGetMemoryBackingBasePath(cfg, &base);
>> -    *path = g_strdup_printf("%s/%s", base, shortName);
>> +    *path = g_strdup_printf("%s/%s-%d", base, shortName, def->id);
>>
>>      return 0;
>>  }
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index b0c0e1a19b..002c092cf8 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -2127,11 +2127,13 @@ qemuDomainSetPrivatePathsOld(virQEMUDriverPtr driver,
>>      virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>>
>>      if (!priv->libDir)
>> -        priv->libDir = g_strdup_printf("%s/domain-%s", cfg->libDir, vm->def->name);
>> +        priv->libDir = g_strdup_printf("%s/domain-%s-%d", cfg->libDir,
>> +                                       vm->def->name, vm->def->id);
>>
>>      if (!priv->channelTargetDir)
>> -        priv->channelTargetDir = g_strdup_printf("%s/domain-%s",
>> -                                                 cfg->channelTargetDir, vm->def->name);
>> +        priv->channelTargetDir = g_strdup_printf("%s/domain-%s-%d",
>> +                                                 cfg->channelTargetDir,
>> +                                                 vm->def->name, vm->def->id);
>>
>>      virObjectUnref(cfg);
>>  }
>> @@ -2150,11 +2152,13 @@ qemuDomainSetPrivatePaths(virQEMUDriverPtr driver,
>>          goto cleanup;
>>
>>      if (!priv->libDir)
>> -        priv->libDir = g_strdup_printf("%s/domain-%s", cfg->libDir, domname);
>> +        priv->libDir = g_strdup_printf("%s/domain-%s-%d", cfg->libDir, domname,
>> +                                       vm->def->id);
>>
>>      if (!priv->channelTargetDir)
>> -        priv->channelTargetDir = g_strdup_printf("%s/domain-%s",
>> -                                                 cfg->channelTargetDir, domname);
>> +        priv->channelTargetDir = g_strdup_printf("%s/domain-%s-%d",
>> +                                                 cfg->channelTargetDir,
>> +                                                 domname, vm->def->id);
>>
>>      ret = 0;
>>   cleanup:
>> --
>> 2.24.1
>>
>
>Regards,
>Daniel
>-- 
>|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200211/8340cce2/attachment-0001.sig>


More information about the libvir-list mailing list