[libvirt] [PATCH] conf: snapshot: Don't default-snapshot empty floppy drives

[Eric Blake]

On 09/11/2014 10:28 AM, Peter Krempa wrote:
> On 09/11/14 18:25, Peter Krempa wrote:
>> On 09/11/14 18:16, Eric Blake wrote:
>>> On 09/11/2014 09:47 AM, Peter Krempa wrote:
>>>> If a floppy drive isn't selected for snapshot explicitly and is empty
>>>> don't try to snapshot it. For external snapshots this would fail as we
>>>> can't generate a name for the snapshot from an empty drive.
>>>
>>> Do we need the same for cdrom drives?
>>
>> CDROMs are automatically read only and thus get the _NONE target right
>> when parsing the configuration.
>>

>>>> +
>>>> +        /* Don't snapshot empty floppy drives */
>>>> +        if (def->dom->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY &&
>>>> +            !virDomainDiskGetSource(def->dom->disks[i]))
>>>
>>> If we are worried about ALL empty drives, it would be simpler to just
>>> drop the left side of the &&, making it solely a test of whether there
>>> is currently a defined host source.
>>>
>>
>> Since only CDROMs and floppies can be empty and cdroms are already
>> exempted from here it should be functionally equivalent to do that. The
>> only limitation is that the check for the empty source probably needs to
>> be stronger (NBD disks may have the disk->src->path NULL even if they
>> have backing.)

<disk type='block' device='lun'> also allows for a NULL src, if I
remember correctly.

> 
> Which reminds me that snapshots of NBD disks are forbidden, so it should
> be fine even without tweaking the emptiness check.

Still feels fragile.

> 
>>
>> I'll post a v2.
>>
> 
> Your call whether I should try to improve the check or leave it as-is.

I'd feel more comfortable with the generic check that all source-less
disks are explicitly tweaked to not have a snapshot taken, rather than
relying on side checks like readonly saving the day.

-- 
Eric Blake   eblake 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: 539 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140911/e696ca4e/attachment-0001.sig>