From: Kevin Wolf <kwolf@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: stefanha@gmail.com, qemu-devel@nongnu.org,
Markus Armbruster <armbru@redhat.com>
Subject: Re: [Qemu-devel] ping Re: [RFC PATCH 00/13] Embedded NBD server
Date: Tue, 18 Sep 2012 11:40:20 +0200 [thread overview]
Message-ID: <50584184.5080602@redhat.com> (raw)
In-Reply-To: <539633110.1300350.1347959367353.JavaMail.root@redhat.com>
Am 18.09.2012 11:09, schrieb Paolo Bonzini:
>>> Luckily removable media are usually not too interesting, so a
>>> slightly suboptimal behavior is okay as long as it does not break the
>>> important use cases---mostly migration without shared storage, where also
>>> uninteresting images have to be mirrored or exposed via NBD. Those
>>> should be covered by bdrv_in_use.
>>
>> It sounds like it could be acceptable, yes. But what's even the
>> motivation to close the server on bdrv_close? The commit message is a
>> bit... well, not just terse, but even empty.
>
> The motivation is two-fold:
>
> 1) for device hot-unplug, not closing the server would impede removal
> of the blockdev until after all clients have closed their connections.
>
> 2) for the removable media case, clients risk reading data from two
> different images and merging it somehow.
>
> In either case (hot-unplug and eject) after bdrv_close I/O requests would
> return ENOMEDIUM, so there is not much benefit in leaving the connection
> open. Clients can reconnect with the understanding that the medium has
> changed (medium change is not part of the NBD specification, but we can
> retrofit it this way).
I think I can buy this, but please add it to the commit message.
>>> Yes, though a lot of these could be moved to "filters" and use
>>> whatever filter-specific method is there (e.g. a filter bdrv_close).
>>> This circles back to the question of whether bdrv_close kills filters
>>> or only the base image...
>>
>> Note that after completing the refactoring, we'll only have one combined
>> bdrv_close/delete function and so there won't be BlockDriverStates
>> that are closed. In this case, I think it's quite obvious that not closing
>> the filters wouldn't make any sense.
>
> Does that mean that any I/O throttling must be applied again on every
> medium change? That would be a behavioral change.
Hm, I guess so, at least on the lowest level. The only thing I know for
certain is that maintaining compatibility for the old commands will be
fun, but if possible at all we shouldn't let that compromise our design.
Kevin
next prev parent reply other threads:[~2012-09-18 9:40 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-27 15:00 [Qemu-devel] [RFC PATCH 00/13] Embedded NBD server Paolo Bonzini
2012-08-27 15:00 ` [Qemu-devel] [RFC PATCH 01/13] nbd: add more constants Paolo Bonzini
2012-08-27 15:00 ` [Qemu-devel] [RFC PATCH 02/13] nbd: pass NBDClient to nbd_send_negotiate Paolo Bonzini
2012-08-27 15:00 ` [Qemu-devel] [RFC PATCH 03/13] nbd: do not leak nbd_trip coroutines when a connection is torn down Paolo Bonzini
2012-08-27 15:00 ` [Qemu-devel] [RFC PATCH 04/13] nbd: close all clients on deleting export Paolo Bonzini
2012-08-27 15:00 ` [Qemu-devel] [RFC PATCH 05/13] nbd: register named exports Paolo Bonzini
2012-08-27 15:00 ` [Qemu-devel] [RFC PATCH 06/13] nbd: negotiate with " Paolo Bonzini
2012-08-27 15:00 ` [Qemu-devel] [RFC PATCH 07/13] nbd: do not close BlockDriverState in nbd_export_close Paolo Bonzini
2012-08-27 15:00 ` [Qemu-devel] [RFC PATCH 08/13] qemu-sockets: publish dummy_opts Paolo Bonzini
2012-08-27 15:00 ` [Qemu-devel] [RFC PATCH 09/13] qmp: add NBD server commands Paolo Bonzini
2012-09-18 20:11 ` Luiz Capitulino
2012-09-19 8:16 ` Paolo Bonzini
2012-08-27 15:00 ` [Qemu-devel] [RFC PATCH 10/13] qemu-sockets: make inet_parse public Paolo Bonzini
2012-08-27 15:00 ` [Qemu-devel] [RFC PATCH 11/13] hmp: add NBD server commands Paolo Bonzini
2012-09-18 20:22 ` Luiz Capitulino
2012-09-19 8:00 ` Paolo Bonzini
2012-08-27 15:00 ` [Qemu-devel] [RFC PATCH 12/13] block: add close notifiers Paolo Bonzini
2012-08-27 15:00 ` [Qemu-devel] [RFC PATCH 13/13] nbd: add notifier to close exports when the image is closed Paolo Bonzini
2012-09-07 15:50 ` [Qemu-devel] ping Re: [RFC PATCH 00/13] Embedded NBD server Paolo Bonzini
2012-09-07 16:11 ` Kevin Wolf
2012-09-17 16:43 ` Paolo Bonzini
2012-09-18 8:45 ` Kevin Wolf
2012-09-18 9:09 ` Paolo Bonzini
2012-09-18 9:40 ` Kevin Wolf [this message]
2012-09-18 9:48 ` Paolo Bonzini
2012-09-18 9:55 ` Kevin Wolf
2012-09-19 10:16 ` [Qemu-devel] " Daniel P. Berrange
2012-09-19 10:22 ` 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=50584184.5080602@redhat.com \
--to=kwolf@redhat.com \
--cc=armbru@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@gmail.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).