[PATCH] Override devices in reference configuration, where the ID is the same

# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1218817591 25200 # Node ID df2834c111fe5cee0ec322d7a4f10a0083638eae # Parent 86d7161daef682ba869fbae74133cfd811f1d306 Override devices in reference configuration, where the ID is the same Currently, we depend on libvirt to return an error if the client's combination of ReferenceConfig and ResourceSettings results in a duplicate device. That error is non-obvious in origin. Since two devices that would result in the same InstanceID cannot be properly represented by the providers anyway, this patch adds some logic to properly override such a conflicting device. To do this, I had to make sure we set the ID in rasd_to_vdev, which wasn't previously required. The actual insertion of the device into the appropriate type list is done by a helper function which checks for (and overrides if appropriate) a duplicate before tacking the new instance on the end of the list. Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r 86d7161daef6 -r df2834c111fe src/Virt_VirtualSystemManagementService.c --- a/src/Virt_VirtualSystemManagementService.c Thu Aug 14 07:28:22 2008 -0700 +++ b/src/Virt_VirtualSystemManagementService.c Fri Aug 15 09:26:31 2008 -0700 @@ -411,6 +411,9 @@ free(dev->dev.net.mac); dev->dev.net.mac = strdup(val); + free(dev->id); + dev->id = strdup(dev->dev.net.mac); + op = CMGetObjectPath(inst, NULL); if (op == NULL) { CU_DEBUG("Unable to get instance path"); @@ -449,6 +452,9 @@ dev->dev.disk.source = strdup(val); dev->dev.disk.disk_type = disk_type_from_file(val); + free(dev->id); + dev->id = strdup(dev->dev.disk.virtual_dev); + return NULL; } @@ -469,6 +475,9 @@ free(dev->dev.disk.source); dev->dev.disk.source = strdup(val); dev->dev.disk.disk_type = DISK_FS; + + free(dev->id); + dev->id = strdup(dev->dev.disk.virtual_dev); return NULL; } @@ -621,6 +630,33 @@ return true; } +static char *add_device_nodup(struct virt_device *dev, + struct virt_device *list, + int max, + int *index) +{ + int i; + + for (i = 0; i < *index; i++) { + struct virt_device *ptr = &list[i]; + + if (STREQC(ptr->id, dev->id)) { + CU_DEBUG("Overriding device %s from refconf", ptr->id); + cleanup_virt_device(ptr); + memcpy(ptr, dev, sizeof(*ptr)); + return NULL; + } + } + + if (*index == max) + return "Internal error: no more device slots"; + + memcpy(&list[*index], dev, sizeof(list[*index])); + *index += 1; + + return NULL; +} + static const char *classify_resources(CMPIArray *resources, const char *ns, struct domain *domain) @@ -678,15 +714,31 @@ &domain->dev_mem[0], ns); } else if (type == CIM_RES_TYPE_DISK) { + struct virt_device dev; + + memset(&dev, 0, sizeof(dev)); msg = rasd_to_vdev(inst, domain, - &domain->dev_disk[domain->dev_disk_ct++], + &dev, ns); + if (msg == NULL) + msg = add_device_nodup(&dev, + domain->dev_disk, + count, + &domain->dev_disk_ct); } else if (type == CIM_RES_TYPE_NET) { + struct virt_device dev; + + memset(&dev, 0, sizeof(dev)); msg = rasd_to_vdev(inst, domain, - &domain->dev_net[domain->dev_net_ct++], + &dev, ns); + if (msg == NULL) + msg = add_device_nodup(&dev, + domain->dev_net, + count, + &domain->dev_net_ct); } if (msg != NULL) return msg;

+static char *add_device_nodup(struct virt_device *dev, + struct virt_device *list, + int max, + int *index) +{ + int i; + + for (i = 0; i < *index; i++) { + struct virt_device *ptr = &list[i]; + + if (STREQC(ptr->id, dev->id)) { + CU_DEBUG("Overriding device %s from refconf", ptr->id); + cleanup_virt_device(ptr); + memcpy(ptr, dev, sizeof(*ptr)); + return NULL; + } + } + + if (*index == max) + return "Internal error: no more device slots";
I don't think this handles the case where the user supplies a disk/net RASD for the ResourceSettings that is different from the disk/net device specified in the ReferencedConfiguration guest. Let say the user only specifies the RefConfig plus an additional net device for the ResourceSettings. The value for count is 1 (because that's the size of the array passed in). The net device isn't a dup, so the value of index is bumped up to 1, and we return an error. Some debug I added: Virt_VirtualSystemManagementService.c(643): trying to add aa:aa:aa:00:00:00 - the value of refconf dev id is 11:22:33:aa:bb:cc Virt_VirtualSystemManagementService.c(652): *index is 1, max is 1 Virt_VirtualSystemManagementService.c(983): Failed to classify resources: Internal error: no more device slots -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com

KR> Let say the user only specifies the RefConfig plus an additional net KR> device for the ResourceSettings. The value for count is 1 (because KR> that's the size of the array passed in). The net device isn't a KR> dup, so the value of index is bumped up to 1, and we return an KR> error. Oh, right, hmm. That must have been broken with refconf before, I think. That scenario worked for me in my testing, but I think I actually lucked out because of the way I was doing my resources. Hrm, I'll have to think about the best way to fix up the list max length. Thanks! -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
participants (2)
-
Dan Smith
-
Kaitlin Rupert