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