On 23.01.2017 13:30, Fam Zheng wrote: > Writing to the same qcow2 file from two QEMU processes at the same time > will never work correctly, so disable it even when the caller specifies > BDRV_O_RDWR. > > Other formats are less vulnerable because they don't have internal > snapshots thus qemu-img is less often misused to create live snapshot.
Hm, OK, reasonable. Also reasonable since we can just wait with those until we have op blockers. Which brings me to the op blocker point. I don't know the exact influence Kevin's patches will have on this series, but I'd imagine they mostly change where the BDRV_O_SHARE_RW flag comes from or whether we need that flag at all. Therefore, I personally don't mind the order in which your series land in master. > Signed-off-by: Fam Zheng <[email protected]> > --- > block/qcow2.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/block/qcow2.c b/block/qcow2.c > index 96fb8a8..879361a 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -1177,6 +1177,17 @@ static int qcow2_open(BlockDriverState *bs, QDict > *options, int flags, > } > } > > + if ((flags & BDRV_O_SHARE_RW) && (flags & BDRV_O_RDWR)) { > + /* Shared write is never a good idea for qcow2, override it. > + * XXX: Use permission propagation and masking mechanism in op > blockers > + * API once it's there. */ > + ret = bdrv_reopen(bs->file->bs, flags & ~BDRV_O_SHARE_RW, > &local_err); > + if (ret) { > + error_propagate(errp, local_err); > + goto fail; > + } > + } > + Good in principle, but I don't think it should be at the bottom of this function, especially not after "Repair image if dirty". I think it would be good to put this right at the start of qcow2_open(), actually. Max > #ifdef DEBUG_ALLOC > { > BdrvCheckResult result = {0}; >
signature.asc
Description: OpenPGP digital signature
