[libvirt] [PATCHv2 2/2] qemu: add new disk device='lun' for bus='virtio' & type='block'

[Eric Blake]

On 01/05/2012 08:59 AM, Laine Stump wrote:

>>> @@ -9607,7 +9609,8 @@ static int
>>> qemuDomainSnapshotIsAllowed(virDomainObjPtr vm)
>>>         * that succeed as well
>>>         */
>>>        for (i = 0; i<   vm->def->ndisks; i++) {
>>> -        if (vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK&&
>>> +        if ((vm->def->disks[i]->device ==
>>> VIR_DOMAIN_DISK_DEVICE_DISK ||
>>> +             vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_LUN)&&
>>>                (!vm->def->disks[i]->driverType ||
>>>                 STRNEQ(vm->def->disks[i]->driverType, "qcow2"))) {
>>>                qemuReportError(VIR_ERR_OPERATION_INVALID,
>> I think that we should fail for device='lun' and format != 'raw'.
> 
> Truthfully, I didn't totally understand what that code was doing, but it
> seemed appropriate to at least duplicate the behavior of DEVICE_DISK :-)
> Looking at it some more, I *think* what is necessary, is to always
> return false if there is a disk that is DEVICE_LUN, since really the
> only thing that can be snapshotted is a qcow2 disk, and DEVICE_LUN disks
> by definition are never qcow2. So does it seem reasonable to change this
> to:
> 
> 
> -        if (vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK&&
> -              (!vm->def->disks[i]->driverType ||
> -              STRNEQ(vm->def->disks[i]->driverType, "qcow2"))) {
> +        if ((vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_LUN) ||
> +             (vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK&&
> +              STRNEQ_NULLABLE(vm->def->disks[i]->driverType, "qcow2"))) {
>               qemuReportError(VIR_ERR_OPERATION_INVALID,

Yes, that would be reasonable.  This function is only called when doing
an internal snapshot, which requires that all disk devices from the
guest perspective are in qcow2 format on the host.  But I agree that a
device='lun' will never be in qcow2 format - if you plan on using raw
SG_IO, then you plan on using the disk as-is (raw) and not wrapping it
through another layer of qemu emulation.  In short, we are safe
requiring that an internal snapshot cannot be done if any device='lun'
are present.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120105/59e9fbaf/attachment-0001.sig>