Re: [libvirt] [PATCH] Reject duplicate disk targets

On 07/10/2013 02:39 PM, Daniel P. Berrange wrote: > On Wed, Jul 10, 2013 at 02:34:30PM +0200, Martin Kletzander wrote: >> We never check for disks with duplicate targets connected to the same >> controller/bus/etc. That means we go ahead, create the same IDs for >> them and pass them to qemu, which subsequently fails to start. >> >> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=968899 >> Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; >> --- >> src/conf/domain_conf.c | 42 +++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 41 insertions(+), 1 deletion(-) >> >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >> index 3398d8b..01720e1 100644 >> --- a/src/conf/domain_conf.c >> +++ b/src/conf/domain_conf.c >> @@ -2629,6 +2629,45 @@ virDomainDeviceInfoIterate(virDomainDefPtr def, >> >> >> static int >> +virDomainDefRejectDuplicateDiskTargets(virDomainDefPtr def) >> +{ >> + char *disk_id = NULL; >> + int ret = -1; >> + size_t i = 0; >> + virHashTablePtr targets = NULL; >> + >> + if (!(targets = virHashCreate(def->ndisks, NULL))) >> + goto cleanup; >> + >> + for (i = 0; i < def->ndisks; i++) { >> + virDomainDiskDefPtr disk = def->disks[i]; >> + >> + if (virAsprintf(&disk_id, "%d%s%d%d%d%d", >> + disk->bus, >> + NULLSTR(disk->dst), >> + disk->info.addr.drive.controller, >> + disk->info.addr.drive.bus, >> + disk->info.addr.drive.target, >> + disk->info.addr.drive.unit) < 0) > > Err, disk->info.addr is a union of many different types of > address. You can't just arbitrarily reference info.addr.drive > without first validating the type of the union. > I jumped straight into that because we know it is disk and we do it the same way when assigning aliases. I'd be glad to see what option I missed, thanks for any pointers.