Re: [Qemu-devel] ping Re: [RFC PATCH 00/13] Embedded NBD server

[Kevin Wolf]

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