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

[Soren Hansen]

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/