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

[Kevin Wolf]

Am 17.09.2012 18:43, schrieb Paolo Bonzini:
> Il 07/09/2012 18:11, Kevin Wolf ha scritto:
>> I was planning to review it in more detail next week, but I just had a
>> quick look. I'm not sure if automatically shutting down the NBD server
>> when the guest stops using it is always right (for removable media it
>> could even be an eject from the guest),
> 
> Yes, the removable media case could be a bit too eager.  Note however
> that a guest-triggered eject doesn't do bdrv_close, only a
> user-triggered eject does, and that's blocked by bdrv_in_use.
> 
> 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 standard case for closing images is that qemu exits. In this case,
the NBD server would automatically exit as well. An interesting case for
the NBD server would be when the migration has completed; but do we even
get a bdrv_close there?

>> but introducing a notifier list
>> doesn't look too bad. We can probably use it for other things that are
>> currently hardcoded in bdrv_close() with some if statements, like
>> disabling I/O throttling, cancelling a block job, etc.
> 
> 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.

Kevin