On Thu, Jul 18, 2013 at 11:32:52AM -0600, Eric Blake wrote: > On 07/18/2013 11:27 AM, Eric Blake wrote: > > >> 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 > > Well, I _DO_ care about one thing - make sure that the qapi-schema.json > page accurately documents how this variable is defaulted for callers > that don't care about the implications of omitting a format. > > Or we could simplify life by making 'format' mandatory for drive-backup; > it was optional for 'drive-mirror' due to incremental implementation, > but for 'drive-backup', we still have the opportunity to do things right > from the first release.
Ah, I did make a doc change, I must have forgotten to add it. I'm all for making format mandatory if that is ok with everyone. Maybe that is the best solution. Ian