Re: [Qemu-devel] [Qemu-block] [PATCH] tcmu: Introduce qemu-tcmu utility

[Kevin Wolf]

Am 12.03.2019 um 03:18 hat Fam Zheng geschrieben:
> > On Mar 11, 2019, at 19:06, Kevin Wolf <address@hidden> wrote:
> > Am 09.03.2019 um 02:46 hat Yaowei Bai geschrieben:
> >> Thanks for explaining the background. It comes to my mind that actually we
> >> talked about these two cases with Fam a bit long time ago and decided to
> >> support both these two cases. The reason why we implement case2 first is
> >> currently we care more about exporting new opened images and it's a bit
> >> more convenient, exporting from a VM or QMP can be added in the later
> >> release. Do you think it's reasonable/acceptable that we support both
> >> cases and use case2 for normal new opened images and case1 for the
> >> circumstances you mention above?
> > 
> > I would like to avoid a second code path because it comes with a
> > maintenance cost.
> > 
> > Experience also tells that adding a new way to parse option strings will
> > come back at us later because it we must always maintain compatibility
> > with yet another format.
> 
> If the rule is that cfgstr strictly follows -blockdev syntax and the
> parsing code is mostly shared, it shouldn’t be a problem, right? I
> suppose this will limit the expressiveness of cfgstr but might be a
> reasonable tradeoff between implementation complexity, user
> friendliness and interface consistency.

Yes, if we could pass the cfgstr directly to an existing parser, that
would make it less bad at least.

Of course, if we directly pass cfgstr to the -blockdev parser, it also
means that we can't have additional options for configuring the
BlockBackend (which could be part of the -export command line option for
qemu-tcmu).

That could be addressed by wrapping another QAPI object around it that
only contain BlockdevOptions as one field, but then you have to prefix
every block node option if you use keyval visitor syntax.

> Another possibility is to use json: format in cfgstr for anything more
> complex than a single -blockdev.

Parsing like -blockdev already includes JSON syntax (which is necessary
to get the full expressiveness).

If you want to support the equivalent of multiple -blockdevs, you'd need
to wrap the BlockdevOptions in a QAPI list.

> > So I would prefer not doing this and just passing command line options
> > to qemu-tcmu, which can reuse the already existing code paths.
> 
> I think the effect of not supporting adding new blockdev from cfgstr
> is that we have to resort to QMP to allow hot-adding targets. It’s not
> necessarily bad, though I’m not sure hwo that aligns with Yaowei’s
> development plan.

This is true, but we need a way to do this from QMP anyway. So the
question is whether we want to introduce a second way to achieve the
same thing a bit more conveniently. But I expect that hot-adding is
something that is usually done with management software anyway, so does
the convenience actually buy us much?

Kevin