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

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

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.