All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>,
	qemu-devel@nongnu.org, Luiz Capitulino <lcapitulino@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 3/3] gtk: add devices menu to allow changing removable block devices
Date: Thu, 02 May 2013 17:29:31 +0200	[thread overview]
Message-ID: <87mwsdien8.fsf@blackfin.pond.sub.org> (raw)
In-Reply-To: <20130502140622.GC30664@dhcp-200-207.str.redhat.com> (Kevin Wolf's message of "Thu, 2 May 2013 16:06:22 +0200")

Kevin Wolf <kwolf@redhat.com> writes:

> Am 02.05.2013 um 15:41 hat Anthony Liguori geschrieben:
>> Kevin Wolf <kwolf@redhat.com> writes:
>> 
>> > Am 26.04.2013 um 21:43 hat Anthony Liguori geschrieben:
[...]
>> >> +
>> >> +        if (strcmp(type, "ide-cd") == 0) {
>> >> +            disk_type = DT_CDROM;
>> >> +        } else if (strcmp(type, "isa-fdc") == 0) {
>> >> +            disk_type = DT_FLOPPY;
>> >> +        } else {
>> >> +            disk_type = DT_NORMAL;
>> >> +        }
>> >
>> > Same thing here, comparing against strings is a hack. Devices should
>> > probably have a property that says what kind of device they are.
>> 
>> Ack, this is nasty.  I would like to eliminate this.  There is a type
>> field in BlockInfo but:
>> 
>> # @type: This field is returned only for compatibility reasons, it should
>> #        not be used (always returns 'unknown')
>> 
>> I vaguely remember this happening but I don't remember the specific
>> reason why.  I would definitely prefer that we filled out type
>> correctly.
>> 
>> I think Markus was involved in this.  Markus or Luiz, do you remember
>> the story here?
>
> The reason is that BlockInfo is about the backend and it simply doesn't
> know (ever since we introduced if=none, this was buggy, so we just
> abandoned it at some point). We would have to ask the device, not the
> block layer.

Correct.

commit d8aeeb31d53a07a0cce36c7bcf53684953c2e445
Author: Markus Armbruster <armbru@redhat.com>
Date:   Mon May 16 15:04:55 2011 +0200

    block QMP: Deprecate query-block's "type", drop info block's "type="
    
    query-block's specification documents response member "type" with
    values "hd", "cdrom", "floppy", "unknown".
    
    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.
    
    No known user of QMP uses "type".  It's unlikely that any unknown
    users exist, because its value is useless unless you know how the
    block device was created.  But then you also know the true value.
    
    Fixing the broken value risks breaking (hypothetical!) clients that
    somehow rely on the current behavior.  Not fixing the value risks
    breaking (hypothetical!) clients that rely on the value to be
    accurate.  Can't entirely avoid hypothetical lossage.  Change the
    value to be always "unknown".
    
    This makes "info block" always report "type=unknown".  Pointless.
    Change it to not report the type.
    
    Signed-off-by: Markus Armbruster <armbru@redhat.com>
    Signed-off-by: Kevin Wolf <kwolf@redhat.com>

  reply	other threads:[~2013-05-02 15:29 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-26 19:43 [Qemu-devel] [PATCH 0/3] gtk: add Devices menu Anthony Liguori
2013-04-26 19:43 ` [Qemu-devel] [PATCH 1/3] ide: add drive-id property Anthony Liguori
2013-05-02 10:05   ` Andreas Färber
2013-05-02 12:59     ` Anthony Liguori
2013-04-26 19:43 ` [Qemu-devel] [PATCH 2/3] monitor: add notifier list for monitor events Anthony Liguori
2013-04-26 19:43 ` [Qemu-devel] [PATCH 3/3] gtk: add devices menu to allow changing removable block devices Anthony Liguori
2013-05-02  8:49   ` Kevin Wolf
2013-05-02 13:41     ` Anthony Liguori
2013-05-02 13:53       ` Luiz Capitulino
2013-05-02 14:06       ` Kevin Wolf
2013-05-02 15:29         ` Markus Armbruster [this message]
2013-05-02 15:40         ` Anthony Liguori
2013-05-03 12:31           ` [Qemu-devel] [libvirt] " Daniel P. Berrange
2013-05-03 13:52             ` Anthony Liguori
2013-05-03 13:57               ` Daniel P. Berrange
2013-05-02 15:47         ` [Qemu-devel] " Anthony Liguori

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87mwsdien8.fsf@blackfin.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=kwolf@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.