On Tue, Jun 25, 2013 at 04:36:53PM +0200, Paolo Bonzini wrote: > Il 25/06/2013 16:06, Kevin Wolf ha scritto: > > Am 25.06.2013 um 15:49 hat Paolo Bonzini geschrieben: > >> Il 25/06/2013 15:26, Kevin Wolf ha scritto: > >>>>> + if (!full) { > >>>>> + error_setg(&errp, "-f is not yet implemented"); > >>>>> + hmp_handle_error(mon, &errp); > >>>>> + return; > >>>>> + } > >>> Then why make it a valid option and confuse users in the help text by > >>> describing options that don't really exist? > >> > >> Because otherwise we're stuck with a meaning of the flag that is > >> different between drive-mirror and block-backup. > > > > Do you mean when "otherwise" isn't only "we don't add -f now", but also > > "we accidentally add a -f with different meaning later"? Not sure if > > there's a real danger of that when we're aware that we want -f with the > > same meaning as for mirroring. > > We have drive-mirror with: > * the default is 'top' > * -f gives 'full' > > block-backup for now only implements 'full'. If we do not force the > user to add -f, the default is 'full' and we should not change it later. > > However, I would move the "not yet implemented" error from HMP to QMP. > This way, both drive-mirror and block-backup will have a mandatory > 'sync' argument. We plan to implement it anyway, and it makes sense imo > to avoid gratuitous differences in the APIs.
Thanks, I should have explained this in the commit message. Requiring -f now avoids changing semantics later when 'top' becomes the default to match drive-mirror. I'll move the error into qmp_drive_backup(). Stefan