On 19-08-2010 15:22, Daniel P. Berrange wrote:
+static inline void umlShrinkDisks(virDomainDefPtr def, size_t i) +{ + if (def->ndisks > 1) { + memmove(def->disks + i, + def->disks + i + 1, + sizeof(*def->disks) * + (def->ndisks - (i + 1))); + def->ndisks--; + if (VIR_REALLOC_N(def->disks, def->ndisks) < 0) { + /* ignore, harmless */ + } + } else { + VIR_FREE(def->disks); + def->ndisks = 0; + } +} Since this code is already used in the QEMU driver, I think we should just put it straight into src/conf/domain_conf.c so we can share it. 'virDomainDiskRemove' is probably a more accurate name than ShrinkDisks.
Sounds good to me. I wasn't sure where to stick it, so I just left it here and figured you'd probably tell me something like this :) I'll move it.
+ + +static int umlDomainAttachUmlDisk(struct uml_driver *driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk) +{ + int i, ret; + char *cmd = NULL; + char *reply; + + for (i = 0 ; i < vm->def->ndisks ; i++) { + if (STREQ(vm->def->disks[i]->dst, disk->dst)) { + umlReportError(VIR_ERR_OPERATION_FAILED, + _("target %s already exists"), disk->dst); + return -1; + } + } + + if (!disk->src) { + umlReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("disk source path is missing")); + goto error; + } + + if (virAsprintf(&cmd, "config %s=%s", disk->dst, disk->src) < 0) { + virReportOOMError(); + return -1; + } + + if (umlMonitorCommand(driver, vm, cmd, &reply) < 0) + return -1;
Needs to be a 'goto error' to avoid leaking 'cmd'
Ah, yes, good catch. I also need to free the reply. :)
@@ -1900,9 +2119,9 @@ static virDriver umlDriver = { umlDomainStartWithFlags, /* domainCreateWithFlags */ umlDomainDefine, /* domainDefineXML */ umlDomainUndefine, /* domainUndefine */ - NULL, /* domainAttachDevice */ + umlDomainAttachDevice, /* domainAttachDevice */ NULL, /* domainAttachDeviceFlags */ - NULL, /* domainDetachDevice */ + umlDomainDetachDevice, /* domainDetachDevice */ NULL, /* domainDetachDeviceFlags */
You should also implement the DeviceFlags variants at the same time. You can just do a dumb wrapper like QEMU driver does, for simplicity.
Right you are. I misunderstood what those were for. -- Soren Hansen Ubuntu Developer http://www.ubuntu.com/