[libvirt] [PATCH 2/3] conf: Check for user aliases duplicates only

Michal Privoznik posted 3 patches 7 years, 2 months ago
[libvirt] [PATCH 2/3] conf: Check for user aliases duplicates only
Posted by Michal Privoznik 7 years, 2 months ago
https://bugzilla.redhat.com/show_bug.cgi?id=1553162

When validating a device XML config we check if user provided
alias is unique. We do this by maintaining a hash table of device
aliases as we iterated over all devices defined for the domain.
However, it may happen that what appears as two devices in domain
XML is in fact just one interface in hypervisor. For instance in
qemu driver this is true for uhci/ehci controllers. In that case
an error is reported even though it is not actually an error. At
any rate, we can assume libvirt generated aliases to be unique
and thus really check user provided ones only.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/conf/domain_conf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b98b1ca42..04a6ee77a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5573,7 +5573,7 @@ virDomainDeviceDefValidateAliasesIterator(virDomainDefPtr def,
     struct virDomainDefValidateAliasesData *data = opaque;
     const char *alias = info->alias;
 
-    if (!alias)
+    if (!alias || !virDomainDeviceAliasIsUserAlias(alias))
         return 0;
 
     /* Some crazy backcompat for consoles. */
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] conf: Check for user aliases duplicates only
Posted by Peter Krempa 7 years, 2 months ago
On Fri, Mar 09, 2018 at 12:56:12 +0100, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1553162
> 
> When validating a device XML config we check if user provided
> alias is unique. We do this by maintaining a hash table of device
> aliases as we iterated over all devices defined for the domain.
> However, it may happen that what appears as two devices in domain
> XML is in fact just one interface in hypervisor. For instance in
> qemu driver this is true for uhci/ehci controllers. In that case
> an error is reported even though it is not actually an error. At
> any rate, we can assume libvirt generated aliases to be unique
> and thus really check user provided ones only.


So I find some parts of this description slightly misleading. I agree
that we should only validate that the user-provided aliases are unique,
since libvirt should do the proper thing otherwise and the user-aliases
are in it's own namespace.

I think though that the hint to the usb controller requiring multiple
devices from the libvirt view map to a single device in qemu, or any
other reason why two libvirt entries should have the same alias is not
future-proof enough. With this commit you make it possible to use it
with the default alias, but it still might be possible to use with a
user alias.

ACK to this patch, since I think it's the correct thing to do, but
please delete the irrelevant blurb about uhci/ehci in qemu.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list