Kevin Wolf <kw...@redhat.com> writes: > Am 10.08.2018 um 19:33 hat Eric Blake geschrieben: >> On 08/10/2018 11:26 AM, Kevin Wolf wrote: >> > The block-commit QMP command required specifying the top and base nodes >> > of the commit jobs using the file name of that node. While this works >> > in simple cases (local files with absolute paths), the file names >> > generated for more complicated setups can be hard to predict. >> > >> > This adds two new options top-node and base-node to the command, which >> > allow specifying node names instead. They are mutually exclusive with >> > the old options. >> > >> > Signed-off-by: Kevin Wolf <kw...@redhat.com> >> > --- >> > qapi/block-core.json | 24 ++++++++++++++++++------ >> > blockdev.c | 32 ++++++++++++++++++++++++++++++-- >> > 2 files changed, 48 insertions(+), 8 deletions(-) >> > >> > diff --git a/qapi/block-core.json b/qapi/block-core.json >> > index 5b9084a394..91dd075c84 100644 >> > --- a/qapi/block-core.json >> > +++ b/qapi/block-core.json [...] >> Or, here's an idea: >> >> Keep the name @base and @top, but add a new '*by-node':'bool' parameter, >> defaulting to false for now, but perhaps with a deprecation warning that >> we'll switch the default to true in one release and delete the parameter >> altogether in an even later release. When false, @base and @top are >> filenames, as before; when true, @base and @top are node names instead. >> Introspectible, nicer names in the long run, and avoids having to detect a >> user providing two mutually-exclusive options at once. > > I don't like options that completely change the semantics of other > options, but maybe that's just personal preference.
I happen to share it. > Anyway, thinking about the long term for block-commit is useless, the > command is just broken (for example, the @device option doesn't make any > sense) and will have to be replaced. But libvirt needs something _now_ > for the -blockdev support, so I decided to add this as a quick hack > before we get the proper replacement. > > I think it makes more sense to create a new blockdev-commit (which > would be a name more in line with the other block job commands) and > deprecate the old block-commit command as a whole. > >> > +++ b/blockdev.c >> > @@ -3308,7 +3308,9 @@ out: >> > } >> > void qmp_block_commit(bool has_job_id, const char *job_id, const char >> > *device, >> > + bool has_base_node, const char *base_node, >> > bool has_base, const char *base, >> > + bool has_top_node, const char *top_node, >> > bool has_top, const char *top, >> > bool has_backing_file, const char *backing_file, >> > bool has_speed, int64_t speed, >> >> Getting to be a long signature. Should we use 'boxed':true in the QAPI file >> to make this easier to write? (Separate commit) > > It's an option. > > Has any progress been made on the plan to support defaults in QAPI, so > that we could get rid of the has_* parameters? No. It's one of those things that keep getting pushed out by more important or urgent stuff. I expect it to be straightforward, if tedious.
