| Message ID | 20220210133123.347350-1-f.ebner@proxmox.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Make qemu-img dd more flexible | expand |
On Thu, Feb 10, 2022 at 02:31:19PM +0100, Fabian Ebner wrote: > Adds support for reading from stdin and writing to stdout (when raw > format is used), as well as overriding the size of the output and > input image/stream. > > Additionally, the options -n for skipping output image creation and -l > for loading a snapshot are made available like for convert. Without looking at the series itself, I want to refer back to earlier times that someone proposed improving 'qemu-img dd': https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg00636.html https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg02618.html As well as the observation that when we originally allowed 'qemu-img dd' to be added, the end goal was that if 'qemu-img dd' can't operate as a thin wrapper around 'qemu-img convert', then 'qemu-img convert' needs to be made more powerful first. Every time we diverge on what the two uses can do, rather than keeping dd as a thin wrapper, we add to our maintenance burden. Sadly, there is a lot of technical debt in this area ('qemu-img dd skip= count=' is STILL broken, more than 4 years after I first proposed a potential patch), where no one has spent the necessary time to improve the situation.
On 11.02.22 17:31, Eric Blake wrote: > On Thu, Feb 10, 2022 at 02:31:19PM +0100, Fabian Ebner wrote: >> Adds support for reading from stdin and writing to stdout (when raw >> format is used), as well as overriding the size of the output and >> input image/stream. >> >> Additionally, the options -n for skipping output image creation and -l >> for loading a snapshot are made available like for convert. > Without looking at the series itself, I want to refer back to earlier > times that someone proposed improving 'qemu-img dd': > > https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg00636.html > https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg02618.html > > As well as the observation that when we originally allowed 'qemu-img > dd' to be added, the end goal was that if 'qemu-img dd' can't operate > as a thin wrapper around 'qemu-img convert', then 'qemu-img convert' > needs to be made more powerful first. Every time we diverge on what > the two uses can do, rather than keeping dd as a thin wrapper, we add > to our maintenance burden. > > Sadly, there is a lot of technical debt in this area ('qemu-img dd > skip= count=' is STILL broken, more than 4 years after I first > proposed a potential patch), where no one has spent the necessary time > to improve the situation. Note that by now (in contrast to 2018), we have FUSE disk exports, and I even have a script that uses them to let you run dd on any image: https://gitlab.com/hreitz/qemu-scripts/-/blob/main/qemu-dd.py Which is nice, because it gives you feature parity with dd, because it simply runs dd. (The main problem with the script is that it lives in that personal repo of mine and so nobody but me knows about it. Suggestions to improve that situation are more than welcome.) Now, the qemu storage daemon does not support loading qcow2 snapshots (as far as I’m aware), which is proposed in patch 4 of this series. But I think that just means that it would be nice if the QSD could support that. Hanna
Am 11.02.22 um 17:42 schrieb Hanna Reitz: > On 11.02.22 17:31, Eric Blake wrote: >> On Thu, Feb 10, 2022 at 02:31:19PM +0100, Fabian Ebner wrote: >>> Adds support for reading from stdin and writing to stdout (when raw >>> format is used), as well as overriding the size of the output and >>> input image/stream. >>> >>> Additionally, the options -n for skipping output image creation and -l >>> for loading a snapshot are made available like for convert. >> Without looking at the series itself, I want to refer back to earlier >> times that someone proposed improving 'qemu-img dd': >> >> https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg00636.html >> https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg02618.html >> >> As well as the observation that when we originally allowed 'qemu-img >> dd' to be added, the end goal was that if 'qemu-img dd' can't operate >> as a thin wrapper around 'qemu-img convert', then 'qemu-img convert' >> needs to be made more powerful first. Every time we diverge on what >> the two uses can do, rather than keeping dd as a thin wrapper, we add >> to our maintenance burden. I'm wondering why it's not actually implemented as a thin wrapper then? The fact that it isn't is (part of) the reason why dd was chosen, as mentioned in the first patch: "While dd and convert have overlapping use cases, `dd` is a simple read/write loop while convert is much more sophisticated and has ways to dealing with holes and blocks of zeroes. Since these typically can't be detected in pipes via SEEK_DATA/HOLE or skipped while writing, dd seems to be the better choice for implementing stdin/stdout streams." Adding the same feature to convert seems much more involved. >> >> Sadly, there is a lot of technical debt in this area ('qemu-img dd >> skip= count=' is STILL broken, more than 4 years after I first >> proposed a potential patch), where no one has spent the necessary time >> to improve the situation. > > Note that by now (in contrast to 2018), we have FUSE disk exports, and I > even have a script that uses them to let you run dd on any image: > > https://gitlab.com/hreitz/qemu-scripts/-/blob/main/qemu-dd.py > > Which is nice, because it gives you feature parity with dd, because it > simply runs dd. Thank you for the suggestion. It's definitely worth considering, although it does add a bit of complexity and we currently don't have FUSE support enabled in our builds. > > (The main problem with the script is that it lives in that personal repo > of mine and so nobody but me knows about it. Suggestions to improve > that situation are more than welcome.) > > Now, the qemu storage daemon does not support loading qcow2 snapshots > (as far as I’m aware), which is proposed in patch 4 of this series. But > I think that just means that it would be nice if the QSD could support > that. I suppose adding a snapshot-load QMP command would be a natural way to add it? > > Hanna > >