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/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list