All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: amit.shah@redhat.com, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 3/5] raw-posix: keep complete control of door locking if possible
Date: Fri, 10 Feb 2012 15:56:32 +0100	[thread overview]
Message-ID: <m3fweispnj.fsf@blackfin.pond.sub.org> (raw)
In-Reply-To: <4F3522EC.90205@redhat.com> (Paolo Bonzini's message of "Fri, 10 Feb 2012 15:00:12 +0100")

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 02/10/2012 01:49 PM, Markus Armbruster wrote:
>> With manage_door off:
>>
>> * cdrom_lock_medium() does nothing.  Thus, guest OS operating the
>>   virtual door lock does not affect the physical door.
>>
>> * cdrom_eject() does nothing.  Thus, guest OS moving the virtual tray
>>   does not affect the physical tray.
>
> That's because manage_door off means "we couldn't get O_EXCL to
> work". udev will affect the physical tray and we cannot really compete
> with it.

Understood.  I'm just describing behavior to make sure I understand your
patch, not criticizing it.

>> Compare to before this patch:
>>
>> * cdrom_lock_medium() fails silently.  Thus, guest OS operating the
>>   virtual door may or may not affect the physical door.  It usually does
>>   unless the CD-ROM is mounted in the host.
>>
>> * cdrom_eject() perror()s when it fails.  Thus, guest OS moving the
>>   virtual tray may or may not affect the physocal tray.  It usually does
>>   unless the CD-ROM is mounted in the host, or udev got its paws on it
>>   (I think).
>>
>> With manage_door on:
>>
>> * cdrom_lock_medium() works exactly as before, except the common failure
>>   mode "CD-ROM is mounted in the host" can't happen.
>>
>> * cdrom_eject() works exactly as before, except the common failure mode
>>   "CD-ROM is mounted in the host" can't happen.
>>
>> Is this correct?
>>
>> If yes, is it what we want?
>
> For virtio-scsi+scsi-block, we definitely want O_EXCL to remove the
> device from udev and the in-kernel poller.  That's because GESN
> reports events only once, and we do not want the host and guest to
> race on receiving events.  But in order to open with O_EXCL we need to
> unmount (GNOME3 doesn't have an unmount menu item anymore, which is
> why I did it myself in patch 5/5).

Program A unmounting a filesystem that some unknown program B mounted
for its own unknown reasons doesn't feel right to me.  Even when program
B sucks as badly as Gnome appears to do.  Especially when program A does
it automatically, silently, and without a way to switch it off.

> For IDE we do not need O_EXCL in principle.  However, using the
> emulated passthrough CD-ROM gets confusing without O_EXCL (to the user
> and to the guest, always; and sometimes, to the host udev as well).
> Basically, the guest tries to lock the disk, but udev runs as root so
> it can unlock it under its feet as soon as you press the button.  With
> O_EXCL and some extra patches (waiting for Luiz's eject event
> discussion), everything kinda works; still confusing to the user, but
> not to the host or VM.
>
> I have some prototype patches to improve the situation on IDE somehow
> (still a far cry from scsi-block), but I'm waiting for Luiz's eject
> event patches to reach a conclusion first.
>
> There is also the case of emulated passthrough SCSI CD-ROM, but I
> don't care much about it.
>
>> Shouldn't the user be able to ask for one
>> of "grab the CD-ROM exclusively, else fail", "grab it if you can",
>> "don't grab it even if you could"?
>
> Perhaps IDE could be made to work cooperatively, but right now it
> doesn't.  But scsi-block should fail if it cannot manage the door
> because it bypasses all the CD-ROM machinery and issues SCSI commands
> directly.

You've made a good case for why we really, really want managed_door on.
With managed_door off, both software and humans basically run around
confused.

Why do we even offer managed_door off then?

And isn't the automatic, *silent* fallback to the rotten user experience
of managed_door off a recipe for support calls?

[...]

  reply	other threads:[~2012-02-10 14:56 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-08 17:37 [Qemu-devel] [PATCH 0/5] Fix CD-ROM door with SCSI passthrough Paolo Bonzini
2012-02-08 17:37 ` [Qemu-devel] [PATCH 1/5] raw-posix: always prefer specific devices to hdev Paolo Bonzini
2012-02-10 12:49   ` Markus Armbruster
2012-02-08 17:37 ` [Qemu-devel] [PATCH 2/5] raw-posix: put Linux fd fields into a union Paolo Bonzini
2012-02-08 17:37 ` [Qemu-devel] [PATCH 3/5] raw-posix: keep complete control of door locking if possible Paolo Bonzini
2012-02-10 12:49   ` Markus Armbruster
2012-02-10 14:00     ` Paolo Bonzini
2012-02-10 14:56       ` Markus Armbruster [this message]
2012-02-10 15:19         ` Paolo Bonzini
2012-02-08 17:37 ` [Qemu-devel] [PATCH 4/5] configure: probe for dbus Paolo Bonzini
2012-02-08 17:37 ` [Qemu-devel] [PATCH 5/5] raw-posix: unmount CD-ROM filesystem via udisks Paolo Bonzini
2012-02-10 12:51   ` Markus Armbruster
2012-02-10 14:20     ` Paolo Bonzini

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=m3fweispnj.fsf@blackfin.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=amit.shah@redhat.com \
    --cc=pbonzini@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.