Re: [Qemu-block] [Qemu-devel] [PATCH] mirror: hold aio_context before bdrv_drain
On Wed, 04/01 12:42, Bin Wu wrote: From: Bin Wu wu.wu...@huawei.com What's the issue are you fixing? I think the coroutine already is running in the AioContext of bs. Fam Signed-off-by: Bin Wu wu.wu...@huawei.com --- block/mirror.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block/mirror.c b/block/mirror.c index 4056164..08372df 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -530,7 +530,9 @@ static void coroutine_fn mirror_run(void *opaque) * mirror_populate runs. */ trace_mirror_before_drain(s, cnt); +aio_context_acquire(bdrv_get_aio_context(bs)); bdrv_drain(bs); +aio_context_release(bdrv_get_aio_context(bs)); cnt = bdrv_get_dirty_count(bs, s-dirty_bitmap); } -- 1.7.12.4
Re: [Qemu-block] block-commit dropping privs
30.03.2015 18:36, Kevin Wolf wrote: Am 27.03.2015 um 18:12 hat Eric Blake geschrieben: On 03/27/2015 09:36 AM, Michael Tokarev wrote: Wonder how to specify cache mode, or should I open these with proper O_DIRECT/O_SYNC/whatever? It looks like it's possible to change O_DIRECT at runtime but not O_SYNC. And the more interesting question is how to do that from shell. Redirections only get you so far in shell; you may need a wrapper C program go get O_DIRECT and/or O_SYNC pre-set. Then again, if you use QMP and pass over the Unix socket, you need a C program anyways. O_DIRECT can be set with fcntl(), so qemu takes care of that. O_SYNC is completely unused on Linux these days, so that shouldn't be a problem either. (Other platforms use it as a misguided attempt of approximating O_DIRECT. We should really error out instead.) So if I'm not mistaken, just having one read-only and one read-write fd should be enough for any configuration in practice. Probably yes. Except the thing doesn't actually work. ;) When flushing changes to the base image, that base image needs to be reopened. So I should convince qemu that the base image of this qcow file is /dev/fdset/foo, not the one recorded in the header. Is it possible? Thanks, /mjt
Re: [Qemu-block] block-commit dropping privs
01.04.2015 12:26, Michael Tokarev пишет: 30.03.2015 18:36, Kevin Wolf wrote: Am 27.03.2015 um 18:12 hat Eric Blake geschrieben: On 03/27/2015 09:36 AM, Michael Tokarev wrote: Wonder how to specify cache mode, or should I open these with proper O_DIRECT/O_SYNC/whatever? It looks like it's possible to change O_DIRECT at runtime but not O_SYNC. And the more interesting question is how to do that from shell. Redirections only get you so far in shell; you may need a wrapper C program go get O_DIRECT and/or O_SYNC pre-set. Then again, if you use QMP and pass over the Unix socket, you need a C program anyways. O_DIRECT can be set with fcntl(), so qemu takes care of that. O_SYNC is completely unused on Linux these days, so that shouldn't be a problem Do you mean it is unused in qemu or in kernel? :) either. (Other platforms use it as a misguided attempt of approximating O_DIRECT. We should really error out instead.) So if I'm not mistaken, just having one read-only and one read-write fd should be enough for any configuration in practice. Yes, O_DIRECT appears to work with fdsets. Probably yes. Except the thing doesn't actually work. ;) When flushing changes to the base image, that base image needs to be reopened. So I should convince qemu that the base image of this qcow file is /dev/fdset/foo, not the one recorded in the header. qemu-system-x86_64: -drive file=w7x64.qcow2,backing_file=/dev/fdset/2,if=none,id=dr,cache=none: \ could not open disk image w7x64.qcow2: Block format 'qcow2' used by device 'dr' \ doesn't support the option 'backing_file' hmm?.. Is it possible? Thanks, /mjt
Re: [Qemu-block] block-commit dropping privs
Am 01.04.2015 um 11:54 hat Michael Tokarev geschrieben: 01.04.2015 12:26, Michael Tokarev пишет: 30.03.2015 18:36, Kevin Wolf wrote: Am 27.03.2015 um 18:12 hat Eric Blake geschrieben: On 03/27/2015 09:36 AM, Michael Tokarev wrote: Wonder how to specify cache mode, or should I open these with proper O_DIRECT/O_SYNC/whatever? It looks like it's possible to change O_DIRECT at runtime but not O_SYNC. And the more interesting question is how to do that from shell. Redirections only get you so far in shell; you may need a wrapper C program go get O_DIRECT and/or O_SYNC pre-set. Then again, if you use QMP and pass over the Unix socket, you need a C program anyways. O_DIRECT can be set with fcntl(), so qemu takes care of that. O_SYNC is completely unused on Linux these days, so that shouldn't be a problem Do you mean it is unused in qemu or in kernel? :) In qemu. We used to use O_DSYNC for cache=writethrough long ago, but nowadays we just issue fdatasync() manually. either. (Other platforms use it as a misguided attempt of approximating O_DIRECT. We should really error out instead.) So if I'm not mistaken, just having one read-only and one read-write fd should be enough for any configuration in practice. Yes, O_DIRECT appears to work with fdsets. Probably yes. Except the thing doesn't actually work. ;) When flushing changes to the base image, that base image needs to be reopened. So I should convince qemu that the base image of this qcow file is /dev/fdset/foo, not the one recorded in the header. qemu-system-x86_64: -drive file=w7x64.qcow2,backing_file=/dev/fdset/2,if=none,id=dr,cache=none: \ could not open disk image w7x64.qcow2: Block format 'qcow2' used by device 'dr' \ doesn't support the option 'backing_file' Overriding the backing file should work like this: -drive file=...,backing.file.filename=/dev/fdset/2 Kevin
Re: [Qemu-block] [Qemu-devel] [PATCH] mirror: hold aio_context before bdrv_drain
On Wed, Apr 01, 2015 at 04:49:39PM +0800, Bin Wu wrote: On 2015/4/1 16:19, Fam Zheng wrote: On Wed, 04/01 12:42, Bin Wu wrote: From: Bin Wu wu.wu...@huawei.com What's the issue are you fixing? I think the coroutine already is running in the AioContext of bs. Fam In the current implementation of bdrv_drain, it should be placed in a critical section as suggested in the comments above the function: Note that unlike bdrv_drain_all(), the caller must hold the BlockDriverState AioContext. However, the mirror coroutine starting with mirror_run doesn't do this. I just found qmp_drive_mirror protects the AioCentext, but it is out of the scope of the mirror coroutine. There are 3 possibilities: 1. qmp_drive_mirror() under QEMU main loop thread. AioContext is held. 2. IOThread aio_poll(). AioContext is held. 3. QEMU main loop thread when IOThread (dataplane) is not used. Here the AioContext is the global qemu_aio_context. We don't need to acquire it explicitly, we're protected by the global mutex. If #3 was a problem then virtio-blk.c's bdrv_aio_writev() would also be a problem, for example. This patch is unnecessary. Stefan pgpXHSgxh5xxV.pgp Description: PGP signature
Re: [Qemu-block] [PATCH v2 0/3] null driver patches
On Wed, Apr 01, 2015 at 09:45:37AM +0800, Fam Zheng wrote: v2: Change patch 01 to use realtime clock, that way we don't need to check the VCPU state. [Stefan] Added Eric's rev-by in 23. Change title to Null block driver in 3. [Eric] The second patch allows testing commit to a null image. The third adds a stanza to MAINTAINERS. Fam Zheng (3): block/null: Latency simulation by adding new option latency-ns block/null: Support reopen MAINTAINERS: Add Fam Zheng as Null block driver maintainer MAINTAINERS | 6 + block/null.c | 66 +++- qapi/block-core.json | 5 +++- 3 files changed, 70 insertions(+), 7 deletions(-) -- 1.9.3 Reviewed-by: Stefan Hajnoczi stefa...@redhat.com pgpvoAt_480IA.pgp Description: PGP signature
[Qemu-block] qemu-img behavior for locating backing files
Kevin, what's the correct behavior for qemu-img and relative paths when creating a new qcow2 file? Example: (in e.g. /home/qemu/build/ or anywhere not /home: ) qemu-img create -f qcow2 base.qcow2 32G qemu-img create -f qcow2 -F qcow2 -b base.qcow2 /home/overlay.qcow2 In 1.7.0., this produces a warning that the base object cannot be found (because it does not exist at that location relative to overlay.qcow2), but qemu-img will create the qcow2 for you regardless. 2.0, 2.1 and 2.2 all will create the image successfully, with no warnings. 2.3-rc1/master as they exist now will emit an error message and create no image. Since this is a change in behavior for the pending release, is this the correct/desired behavior?
Re: [Qemu-block] [Qemu-devel] qemu-img behavior for locating backing files
On 04/01/2015 10:16 AM, John Snow wrote: Kevin, what's the correct behavior for qemu-img and relative paths when creating a new qcow2 file? Example: (in e.g. /home/qemu/build/ or anywhere not /home: ) qemu-img create -f qcow2 base.qcow2 32G creates /home/qemu/build/base.qcow2 qemu-img create -f qcow2 -F qcow2 -b base.qcow2 /home/overlay.qcow2 Tries to create /home/overlay.qcow2; requires /home/base.qcow2 to exist for the creation to be well-formed. (Any use of /home/qemu/build/base.qcow2 should be wrong) If you want, you could do: qemu-img create -f qcow2 /home/overlay.qcow2 $size qemu-img rebase -u -f qcow2 -F qcow2 -b base.qcow2 /home/overlay.qcow2 to create the file that would relatively point to /home/base.qcow2, whether or not that file already exists; and it could be argued that we may even want to support that via a single create command (that is, 'create an image with this string as the backing file, but without actually chasing through that string') In 1.7.0., this produces a warning that the base object cannot be found (because it does not exist at that location relative to overlay.qcow2), but qemu-img will create the qcow2 for you regardless. Sounds almost ideal (or at least an argument for the 'create with an unsafe string for backing) - but how did it pick the size for that image? 2.0, 2.1 and 2.2 all will create the image successfully, with no warnings. Oops. 2.3-rc1/master as they exist now will emit an error message and create no image. Sounds like a bug fix, not a regression. Since this is a change in behavior for the pending release, is this the correct/desired behavior? Yes, I think so. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-block] [Qemu-devel] [PATCH] mirror: hold aio_context before bdrv_drain
On 2015/4/1 19:59, Stefan Hajnoczi wrote: On Wed, Apr 01, 2015 at 04:49:39PM +0800, Bin Wu wrote: On 2015/4/1 16:19, Fam Zheng wrote: On Wed, 04/01 12:42, Bin Wu wrote: From: Bin Wu wu.wu...@huawei.com What's the issue are you fixing? I think the coroutine already is running in the AioContext of bs. Fam In the current implementation of bdrv_drain, it should be placed in a critical section as suggested in the comments above the function: Note that unlike bdrv_drain_all(), the caller must hold the BlockDriverState AioContext. However, the mirror coroutine starting with mirror_run doesn't do this. I just found qmp_drive_mirror protects the AioCentext, but it is out of the scope of the mirror coroutine. There are 3 possibilities: 1. qmp_drive_mirror() under QEMU main loop thread. AioContext is held. 2. IOThread aio_poll(). AioContext is held. 3. QEMU main loop thread when IOThread (dataplane) is not used. Here the AioContext is the global qemu_aio_context. We don't need to acquire it explicitly, we're protected by the global mutex. If #3 was a problem then virtio-blk.c's bdrv_aio_writev() would also be a problem, for example. This patch is unnecessary. Stefan OK, I see. Thanks -- Bin Wu
