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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
>> ---
>>  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.)

I'll post a v2.

Peter


Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]