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

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