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