[libvirt] [PATCH v1 09/14] virDomainDeviceInfoCheckABIStability: Check for alias too

[Martin Kletzander]

On Thu, Oct 19, 2017 at 01:27:11PM +0100, Daniel P. Berrange wrote:
>On Thu, Oct 19, 2017 at 02:16:41PM +0200, Martin Kletzander wrote:
>> On Thu, Oct 19, 2017 at 10:11:04AM +0200, Michal Privoznik wrote:
>> > Since we'll be passing user's input onto qemu command line, we
>> > have to make sure aliases don't change during migration and all
>> > the other places where ABI is checked. Aliases are part of ABI
>> > now.
>> >
>>
>> I don't see how change of an alias would have an impact on the guest OS since it
>> is not visible at all.  Is QEMU using the IDs to refer to objects in the
>> migrated data stream?  AFAIK (actually not me, I just asked random stranger
>> about it) it does not (except the memory modules that Peter told us about in one
>> of previous series).  I see no reason why we would disable the option to rename
>> a device when doing migration.  We are reconstructing the command-line anyway.
>> Did migration fail for you when you tried this?
>
>QEMU never includes the device ID in the migration stream, except for that
>one screw-up wrt memory modules that we know about.
>
>So technically the change below could be loosened to only apply to the
>memory module alias. Since the whole point of aliases is to have a long
>term stable identifier for devices though, I figure it is probably
>beneficial to enforce no-renames for all devices even if QEMU is safe
>without it.
>

Well, depends how you define long-term.  Since it is the user/mgmt app that
defines these aliases, I don't see why they could not change them.

>>
>> > Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> > ---
>> > src/conf/domain_conf.c | 7 +++++++
>> > 1 file changed, 7 insertions(+)
>> >
>> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> > index 0cf67dff1..cb80939af 100644
>> > --- a/src/conf/domain_conf.c
>> > +++ b/src/conf/domain_conf.c
>> > @@ -19870,6 +19870,13 @@ virDomainDeviceInfoCheckABIStability(virDomainDeviceInfoPtr src,
>> >         return false;
>> >     }
>> >
>> > +    if (STRNEQ_NULLABLE(src->alias, dst->alias)) {
>> > +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> > +                       _("Target device alias %s does not match source %s"),
>> > +                       NULLSTR(src->alias), NULLSTR(dst->alias));
>> > +        return false;
>> > +    }
>> > +
>> >     switch ((virDomainDeviceAddressType) src->type) {
>> >     case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
>> >         if (src->addr.pci.domain != dst->addr.pci.domain ||
>
>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: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20171019/29cfb762/attachment-0001.sig>