Re: [Qemu-devel] [PATCH v3 3/6] block QMP: Drop query-block member "type" (type= in info block)

[Luiz Capitulino]

On Thu, 12 May 2011 19:54:40 +0200
Markus Armbruster <address@hidden> wrote:

> Luiz Capitulino <address@hidden> writes:
> 
> > On Thu, 12 May 2011 19:12:56 +0200
> > Markus Armbruster <address@hidden> wrote:
> >
> >> Luiz Capitulino <address@hidden> writes:
> >> 
> >> > On Thu, 12 May 2011 17:05:12 +0200
> >> > Markus Armbruster <address@hidden> wrote:
> >> >
> >> >> Its value is unreliable: a block device used as floppy has type
> >> >> "floppy" if created with if=floppy, but type "hd" if created with
> >> >> if=none.
> >> >> 
> >> >> That's because with if=none, the type is at best a declaration of
> >> >> intent: the drive can be connected to any guest device.  Its type is
> >> >> really the guest device's business.  Reporting it here is wrong.
> >> >
> >> > It reports how the guest is using the device, right? I'd say that's what
> >> > users/clients are interested in knowing.
> >> 
> >> The value is *unreliable*.  It may or may not match how the guest is
> >> using the device.  I doubt users are interested in unreliable
> >> information.
> >
> > Can't it be fixed? And how are users/clients supposed to find out how
> > the guest is using its block devices?
> 
> To find out more about the guest's devices, examine the guest's devices:
> info qtree.

Which is not converted to QMP yet.

> You don't expect to find the guest serial devices in in "info chardev",
> either.  query-block's type member is a mistake, because it mixes up
> guest device info with the host device info.  Dropping it is a bug fix.

I understand why you're dropping it, what I don't want to do is to break
working clients. For example, there might be clients out there using it
in a way that it's expected to work (eg. if=floppy).

Of course that I'm assuming that such a client exist.

> The fact that its value is unreliable is merely icing on the cake.
> 
> >> > Also, we can't just drop it from QMP. We should first note it's 
> >> > deprecated.
> >> 
> >> Would you accept a change to the more honest value "unknown" for the
> >> deprecation period?
> >
> > We have to avoid breaking the protocol. Changing something that has always
> > been reported as 'cdrom' to 'unknown' will likely cause as many as damages
> > as dropping the command.
> 
> I can cause damage only if somebody is using it.  Which I doubt.

Me too and I'd agree with this patch if I was 100% sure. But it's impossible
to be sure, unless we do it by trial and error which is harmful.

> Remember, the value is unreliable.  It's a *lie*.  We can stop lying in
> two ways: shut up (drop member "type"), or tell the truth (change the
> value to "unknown", which is a documented value of "type").

Can we set it to 'unknown' when if=none?

> > The best solution I can think of is noting in the documentation that the
> > information is unreliable and explain what clients interested in knowing
> > this info should do.
> 
> I'd be much more willing to jump through compatibility hoops if there
> was *one* known user of this particular detail of QMP.

Ideally, yes, but it's the type of thing we'll never know.

> But if you insist on us continuing to lie, I'll find a way to continue
> to lie.  I'm resisting it, because I think it's a disservice to our
> users.

I also want to do the best for our users and I don't want to ignore the bug,
but we don't know whether there are clients using the field. If we drop it
and a client does use it, then we'll have failed in doing the best.