On Tue, Mar 29, 2011 at 11:40:44AM +0900, KAMEZAWA Hiroyuki wrote:
On Tue, 29 Mar 2011 10:41:57 +0800 Hu Tao <hutao@cn.fujitsu.com> wrote:
On Fri, Mar 25, 2011 at 05:17:03PM +0900, KAMEZAWA Hiroyuki wrote:
From 638341bdf3eaac824e36d265e134608279750049 Mon Sep 17 00:00:00 2001 From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Date: Fri, 25 Mar 2011 17:10:58 +0900 Subject: [PATCHv7 3/4] libvirt/qemu - check address confliction before addition.
qemuDomainAttachDevicePersistent() calls qemuDomainAssignPCIAddresses() and virDomainDefAddImplicitControllers() at the end of its call.
But PCI/Drive address confliction checks are PCI - confliction will be found but error report is not verbose. Drive - never done.
For example, you can add following (unusual) 2 devices without errors.
<disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/var/lib/libvirt/images/test3.img'/> <target dev="sdx" bus='scsi'/> <address type='drive' controller='0' bus='0' unit='0'/> </disk>
<disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/var/lib/libvirt/images/test3.img'/> <target dev="sdy" bus='scsi'/> <address type='drive' controller='0' bus='0' unit='0'/> </disk>
It's better to check drive address confliction before addition.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> --- src/conf/domain_conf.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 9 +++++++ 4 files changed, 71 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3e3f342..4a54f62 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1287,6 +1287,65 @@ void virDomainDefClearDeviceAliases(virDomainDefPtr def) virDomainDeviceInfoIterate(def, virDomainDeviceInfoClearAlias, NULL); }
+static int virDomainDeviceAddressMatch(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceInfoPtr info, + void *opaque) +{ + virDomainDeviceInfoPtr checked = opaque; + /* skip to check confliction of alias */ + if (info->type != checked->type) + return 0; + if (info->alias && checked->alias && strcmp(info->alias, checked->alias))
!STREQ instead of strcmp
ok.
+ return -1; + if (!memcmp(&info->addr, &checked->addr, sizeof(info->addr)))
Is it safe to memcmp an union like this? In the cases members of an union are of different sizes, and we intent to memcmp an union member that has a smaller size than the other members, then data in space not used by the union member to be compared is also compared. This is not a desired result.
As far as I checked, it's zero cleared at allocation. Hmm, making this function bigger ?
Yes it is safe if zero-cleared. Not worth to make this function complicated. -- Thanks, Hu Tao