On Thu, Jul 18, 2013 at 11:27:21AM -0600, Eric Blake wrote: > On 07/17/2013 02:04 PM, Ian Main wrote: > > qcow2 supports backing files so it makes sense to default to qcow2 > > for MIRROR_SYNC_MODE_NONE so that we can use the source as a backing > > drive and export it via nbd. Defaulting FULL and TOP to SYNC_MODE_NONE > > breaks tests but that could be fixed if we wanted it. > > > > Signed-off-by: Ian Main <im...@redhat.com> > > --- > > blockdev.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/blockdev.c b/blockdev.c > > index 000dea6..805b0e5 100644 > > --- a/blockdev.c > > +++ b/blockdev.c > > @@ -1462,7 +1462,7 @@ void qmp_drive_backup(const char *device, const char > > *target, > > } > > > > if (!has_format) { > > - format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : > > bs->drv->format_name; > > + format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : "qcow2"; > > Is this the right thing to do? Or should we do: > > if (!has_format) { > if (mode == NEW_IMAGE_MODE_EXISTING) { > format = NULL; > } else { > format = bs->drv->format_name ?: "qcow2"; > } > } > > That is, I think we should default to doing a backup in the format given > by the original (what if the original is qed, which also supports > backing files), and only use qcow2 when there is no guidance whatsoever. > > But in practice, I don't care - format probing is a security hole, so > libvirt should always be passing a format, at which point the entire > !has_format condition should be skipped when called by libvirt.
Heh, actually that is basically what I have now, as with the doc change I forgot to git add it. Doh! I'll repost.. I'm also not opposed to format being non-optional. Ian