[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command |
Date: |
Tue, 31 Jul 2012 12:25:23 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120605 Thunderbird/13.0 |
Am 31.07.2012 12:02, schrieb Paolo Bonzini:
> Il 31/07/2012 11:46, Kevin Wolf ha scritto:
>>>> I'm not even sure about the QMP mirror command itself.
>>>>
>>>> I don't really like it, it does too many things at once: It can create
>>>> the target image file, it opens the target and it actually starts the
>>>> mirroring. It's rather bad at the first two steps, because it doesn't
>>>> take any options. This means that it can't create qcow2v3 images, for
>>>> example. Or you can't mirror into a backup with cache=unsafe while
>>>> running your real VM on cache=writethrough.
>>>
>>> Yes, though this can be worked around with mode: 'existing'.
>>
>> True. Only the problem with image creation, though, not the one with
>> bdrv_open() flags, right?
>
> Yeah, but do you really care about for example io=threads vs. io=native?
> The only interesting one is cache=unsafe; the mirror should enable
> writeback caching on the target (bdrv_swap will disable it if needed;
> I'll change this in the next submission), so cache=writethrough vs.
> writeback doesn't matter.
Can we really make it writeback unconditionally? For a passive mirror it
probably doesn't make a difference, but what happens when the user stops
the mirroring and switches to the target? Will it stay writeback?
The same goes for aio=native/threads. Probably not interesting for the
mirror, but well afterwards.
Another interesting thing is I/O throttling. The mirror currently
implements rate limiting itself, but is there really a reason why we
can't reuse regular I/O throttling on the target?
>>>> Having an all-in-one mirror command is a nice feature for HMP, but for
>>>> QMP it's more like a design problem.
>>>>
>>>> Now I see you have called it drive-mirror
>>>
>>> I thought this was your idea. :)
>>
>> Hm, then probably we discussed similar things before. :-)
>>
>>>> , so that kind of implies that
>>>> it's not the final blockdev-mirror but just a QMP version of a command
>>>> primarily designed for HMP. As such this restricted functionality may be
>>>> acceptable, but it's not like everything is already perfect and there's
>>>> no room for discussion.
>>>
>>> We keep going back to the same point that we do not have -blockdev, but
>>> it's becoming a bit frustrating to always rehash this same point...
>>
>> The question is whether we need it at all. We do have a drive_add
>> if=none, and for creating a mirror target that should actually be enough.
>
> But not for creating images. That would require qemu-img invocation.
Yeah, either qemu-img or another monitor command. I believe that in
practice libvirt will do this anyway if this is the only way to specify
image creation options.
> If you're okay with always using an existing image in the QMP case (and
> moving image creation to the HMP implementation), we can do it. But I'm
> not sure I like it, I think it's excessive in the other direction.
If you think it's helpful, we could make it optional and have a mode
'blockdev' where you don't specify a file name but a blockdev name. But
this is an approach that feels a bit HMPish...
Kevin
- [Qemu-devel] [PATCH 18/47] block: live snapshot documentation tweaks, (continued)
- [Qemu-devel] [PATCH 18/47] block: live snapshot documentation tweaks, Paolo Bonzini, 2012/07/24
- [Qemu-devel] [PATCH 22/47] block: make device optional in BlockInfo, Paolo Bonzini, 2012/07/24
- [Qemu-devel] [PATCH 30/47] mirror: implement completion, Paolo Bonzini, 2012/07/24
- [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command, Paolo Bonzini, 2012/07/24
- Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command, Eric Blake, 2012/07/26
- Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command, Kevin Wolf, 2012/07/31
- Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command, Paolo Bonzini, 2012/07/31
- Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command, Kevin Wolf, 2012/07/31
- Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command, Paolo Bonzini, 2012/07/31
- Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command,
Kevin Wolf <=
- Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command, Paolo Bonzini, 2012/07/31
- Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command, Kevin Wolf, 2012/07/31
- Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command, Paolo Bonzini, 2012/07/31
- Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command, Kevin Wolf, 2012/07/31
- Re: [Qemu-devel] [PATCH 28/47] qmp: add drive-mirror command, Paolo Bonzini, 2012/07/31
[Qemu-devel] [PATCH 21/47] block: add bdrv_ensure_backing_file, Paolo Bonzini, 2012/07/24
[Qemu-devel] [PATCH 35/47] qemu-iotests: add testcases for mirroring on-source-error/on-target-error, Paolo Bonzini, 2012/07/24
[Qemu-devel] [PATCH 24/47] block: introduce new dirty bitmap functionality, Paolo Bonzini, 2012/07/24
[Qemu-devel] [PATCH 45/47] mirror: add buf-size argument to drive-mirror, Paolo Bonzini, 2012/07/24
[Qemu-devel] [PATCH 25/47] block: add block-job-complete, Paolo Bonzini, 2012/07/24