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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list