Re: [libvirt] [PATCH] Support virDomainAttachDevice and virDomainDetachDevice for disks in UML

> +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.

> + > + > +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'

> @@ -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.