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

[Peter Krempa]

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.
> 
>>
>>>
>>> Reported-by: Pavel Hrdina <phrdina at redhat.com>
>>> ---
>>>  src/conf/snapshot_conf.c | 9 ++++++++-
>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
>>> index c53a66b..cbaff74 100644
>>> --- a/src/conf/snapshot_conf.c
>>> +++ b/src/conf/snapshot_conf.c
>>> @@ -561,7 +561,14 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,
>>>          if (VIR_STRDUP(disk->name, def->dom->disks[i]->dst) < 0)
>>>              goto cleanup;
>>>          disk->index = i;
>>> -        disk->snapshot = def->dom->disks[i]->snapshot;
>>> +
>>> +        /* 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.)

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

> 
> I'll post a v2.
> 

Your call whether I should try to improve the check or leave it as-is.

Peter

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