| Message ID | 20180130063433.11605-3-famz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jan 30, 2018 at 02:34:33PM +0800, Fam Zheng wrote: > Signed-off-by: Fam Zheng <famz@redhat.com> > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > qemu-img.texi | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/qemu-img.texi b/qemu-img.texi > index 60a0e080c6..ec7e2f5d1e 100644 > --- a/qemu-img.texi > +++ b/qemu-img.texi > @@ -86,6 +86,13 @@ exclusive with the @var{-O} parameters. It is currently required to also use > the @var{-n} parameter to skip image creation. This restriction may be relaxed > in a future release. > > +@item --force-share (-U) > +If specified, @code{qemu-img} will open the image in shared mode, allowing > +concurrent writers. This wording confuses me. It makes me think of multiple processes writing to the image at the same time... > This option is only allowed when opening images in read-only mode. ...but later it turns out "concurrent writers" means there can be at most 1 writing process and multiple reading processes. How about merging the two statements? "If specified, allows @code{qemu-img} to open the image in read-only mode even if another process already has it open for writing."
On 01/30/2018 12:34 AM, Fam Zheng wrote: > Signed-off-by: Fam Zheng <famz@redhat.com> > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > qemu-img.texi | 7 +++++++ > 1 file changed, 7 insertions(+) > Reviewed-by: Eric Blake <eblake@redhat.com>
On 01/30/2018 12:34 AM, Fam Zheng wrote: > Signed-off-by: Fam Zheng <famz@redhat.com> > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > qemu-img.texi | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/qemu-img.texi b/qemu-img.texi > index 60a0e080c6..ec7e2f5d1e 100644 > --- a/qemu-img.texi > +++ b/qemu-img.texi > @@ -86,6 +86,13 @@ exclusive with the @var{-O} parameters. It is currently required to also use > the @var{-n} parameter to skip image creation. This restriction may be relaxed > in a future release. > > +@item --force-share (-U) > +If specified, @code{qemu-img} will open the image in shared mode, allowing > +concurrent writers. For example, this can be used to get the image information Actually, we only permit one writer at a time. Would it be better to say "allowing a concurrent writer"? > +(with 'info' subcommand) when the image is used by a running guest. Note that > +this could produce inconsistent results because of concurrent metadata changes, > +etc. This option is only allowed when opening images in read-only mode. After all, we are stating that this process (which must be read-only, because we can't have two writers at once) is permitting some other process to be the concurrent writer (but not multiple processes to be concurrent writers)
On Tue, Jan 30, 2018 at 08:23:50AM -0600, Eric Blake wrote: > On 01/30/2018 12:34 AM, Fam Zheng wrote: [...] > > +If specified, @code{qemu-img} will open the image in shared mode, allowing > > +concurrent writers. For example, this can be used to get the image information > > Actually, we only permit one writer at a time. Would it be better to > say "allowing a concurrent writer"? Definitely worth rewording. Otherwise the two sentences: "If specified, @code{qemu-img} will open the image in shared mode, allowing concurrent writers." "This option is only allowed when opening images in read-only mode." are at odds with each other --- it says "concurrent writers" are allowed, BUT "allowed only when opening images in 'read-only' mode". > > +(with 'info' subcommand) when the image is used by a running guest. Note that > > +this could produce inconsistent results because of concurrent metadata changes, > > +etc. This option is only allowed when opening images in read-only mode. > > After all, we are stating that this process (which must be read-only, > because we can't have two writers at once) is permitting some other > process to be the concurrent writer (but not multiple processes to be > concurrent writers) Precisely. So it's worth being clearer. With the rewording suggested by Eric: Reviewed-by: Kashyap Chamarthy <kchamart@redhat.com>
Am 30.01.2018 um 15:23 hat Eric Blake geschrieben: > On 01/30/2018 12:34 AM, Fam Zheng wrote: > > Signed-off-by: Fam Zheng <famz@redhat.com> > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > --- > > qemu-img.texi | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/qemu-img.texi b/qemu-img.texi > > index 60a0e080c6..ec7e2f5d1e 100644 > > --- a/qemu-img.texi > > +++ b/qemu-img.texi > > @@ -86,6 +86,13 @@ exclusive with the @var{-O} parameters. It is currently required to also use > > the @var{-n} parameter to skip image creation. This restriction may be relaxed > > in a future release. > > > > +@item --force-share (-U) > > +If specified, @code{qemu-img} will open the image in shared mode, allowing > > +concurrent writers. For example, this can be used to get the image information > > Actually, we only permit one writer at a time. Would it be better to > say "allowing a concurrent writer"? As far as qemu-img is concerned, multiple writers to the image are allowed. There may be further restrictions imposed by a writer so that a second writer isn't allowed, but with raw images and share-rw=on nothing prevents two qemu instances writing to the same image while qemu-img is accessing it read-only. Kevin
On Tue, Jan 30, 2018 at 05:38:20PM +0100, Kevin Wolf wrote: > Am 30.01.2018 um 15:23 hat Eric Blake geschrieben: > > On 01/30/2018 12:34 AM, Fam Zheng wrote: > > > Signed-off-by: Fam Zheng <famz@redhat.com> > > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > > --- > > > qemu-img.texi | 7 +++++++ > > > 1 file changed, 7 insertions(+) > > > > > > diff --git a/qemu-img.texi b/qemu-img.texi > > > index 60a0e080c6..ec7e2f5d1e 100644 > > > --- a/qemu-img.texi > > > +++ b/qemu-img.texi > > > @@ -86,6 +86,13 @@ exclusive with the @var{-O} parameters. It is currently required to also use > > > the @var{-n} parameter to skip image creation. This restriction may be relaxed > > > in a future release. > > > > > > +@item --force-share (-U) > > > +If specified, @code{qemu-img} will open the image in shared mode, allowing > > > +concurrent writers. For example, this can be used to get the image information > > > > Actually, we only permit one writer at a time. Would it be better to > > say "allowing a concurrent writer"? > > As far as qemu-img is concerned, multiple writers to the image are > allowed. There may be further restrictions imposed by a writer so that a > second writer isn't allowed, but with raw images and share-rw=on nothing > prevents two qemu instances writing to the same image while qemu-img is > accessing it read-only. I agree with what you wrote and it's technically correct, but this is a confusing place to mention concurrent writers. They are a common source of user error and that's why locking was introduced in the first place. There should be a separate paragraph in docs/qemu-block-drivers.texi explaining that share-rw=on can be used safely with format=raw if the guests are configured to safely access a shared disk. It should also mention that share-rw=on is unsafe for image formats. Stefan
Am 31.01.2018 um 15:12 hat Stefan Hajnoczi geschrieben: > On Tue, Jan 30, 2018 at 05:38:20PM +0100, Kevin Wolf wrote: > > Am 30.01.2018 um 15:23 hat Eric Blake geschrieben: > > > On 01/30/2018 12:34 AM, Fam Zheng wrote: > > > > Signed-off-by: Fam Zheng <famz@redhat.com> > > > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > > > --- > > > > qemu-img.texi | 7 +++++++ > > > > 1 file changed, 7 insertions(+) > > > > > > > > diff --git a/qemu-img.texi b/qemu-img.texi > > > > index 60a0e080c6..ec7e2f5d1e 100644 > > > > --- a/qemu-img.texi > > > > +++ b/qemu-img.texi > > > > @@ -86,6 +86,13 @@ exclusive with the @var{-O} parameters. It is currently required to also use > > > > the @var{-n} parameter to skip image creation. This restriction may be relaxed > > > > in a future release. > > > > > > > > +@item --force-share (-U) > > > > +If specified, @code{qemu-img} will open the image in shared mode, allowing > > > > +concurrent writers. For example, this can be used to get the image information > > > > > > Actually, we only permit one writer at a time. Would it be better to > > > say "allowing a concurrent writer"? > > > > As far as qemu-img is concerned, multiple writers to the image are > > allowed. There may be further restrictions imposed by a writer so that a > > second writer isn't allowed, but with raw images and share-rw=on nothing > > prevents two qemu instances writing to the same image while qemu-img is > > accessing it read-only. > > I agree with what you wrote and it's technically correct, but this is a > confusing place to mention concurrent writers. They are a common source > of user error and that's why locking was introduced in the first place. So you understand "concurrent writers" as at least two writers? Because I wouldn't understood it as a writer that is concurrent to the (read-only) qemu-img. Maybe rephrase it as "...allowing other processes to write to the image" then? > There should be a separate paragraph in docs/qemu-block-drivers.texi > explaining that share-rw=on can be used safely with format=raw if the > guests are configured to safely access a shared disk. It should also > mention that share-rw=on is unsafe for image formats. share-rw=on is a -device option and only about the guest, not about the backend. It is never unsafe if the guest can cope with external writers. It just doesn't prevent qemu from locking the image file for image formats. Kevin
On Wed, Jan 31, 2018 at 03:22:49PM +0100, Kevin Wolf wrote: > Am 31.01.2018 um 15:12 hat Stefan Hajnoczi geschrieben: > > There should be a separate paragraph in docs/qemu-block-drivers.texi > > explaining that share-rw=on can be used safely with format=raw if the > > guests are configured to safely access a shared disk. It should also > > mention that share-rw=on is unsafe for image formats. > > share-rw=on is a -device option and only about the guest, not about the > backend. It is never unsafe if the guest can cope with external writers. > It just doesn't prevent qemu from locking the image file for image > formats. Thanks for the explanation. Documentation on share-rw=on|off would be nice. Maybe something like: By default the guest has exclusive write access to its disk image. This is enforced by QEMU via file locking. If the guest can safely share the disk image with other writers the -device ...,share-rw=on parameter can be used. This is only safe if the guest is running software, such as a cluster file system, that coordinates disk accesses to avoid corruption. Note that share-rw=on only declares the guest's ability to share the disk. Some QEMU features, such as image file formats, require exclusive write access to the disk image and this is unaffected by the share-rw=on option. For anyone else reading this, here is the code that protects qcow2 and other image formats: void bdrv_format_default_perms(BlockDriverState *bs, BdrvChild *c, const BdrvChildRole *role, BlockReopenQueue *reopen_queue, uint64_t perm, uint64_t shared, uint64_t *nperm, uint64_t *nshared) { ... /* bs->file always needs to be consistent because of the metadata. We * can never allow other users to resize or write to it. */ if (!(flags & BDRV_O_NO_IO)) { perm |= BLK_PERM_CONSISTENT_READ; } shared &= ~(BLK_PERM_WRITE | BLK_PERM_RESIZE); Stefan
On Thu, 02/01 11:44, Stefan Hajnoczi wrote: > On Wed, Jan 31, 2018 at 03:22:49PM +0100, Kevin Wolf wrote: > > Am 31.01.2018 um 15:12 hat Stefan Hajnoczi geschrieben: > > > There should be a separate paragraph in docs/qemu-block-drivers.texi > > > explaining that share-rw=on can be used safely with format=raw if the > > > guests are configured to safely access a shared disk. It should also > > > mention that share-rw=on is unsafe for image formats. > > > > share-rw=on is a -device option and only about the guest, not about the > > backend. It is never unsafe if the guest can cope with external writers. > > It just doesn't prevent qemu from locking the image file for image > > formats. > > Thanks for the explanation. Documentation on share-rw=on|off would be > nice. > > Maybe something like: > > By default the guest has exclusive write access to its disk image. > This is enforced by QEMU via file locking. If the guest can safely > share the disk image with other writers the -device ...,share-rw=on > parameter can be used. This is only safe if the guest is running > software, such as a cluster file system, that coordinates disk > accesses to avoid corruption. > > Note that share-rw=on only declares the guest's ability to share the > disk. Some QEMU features, such as image file formats, require > exclusive write access to the disk image and this is unaffected by the > share-rw=on option. I will add these paragraphs to the "image locking" seciton in docs/qemu-block-drivers.texi. Fam
On Thu, 02/01 11:44, Stefan Hajnoczi wrote: > On Wed, Jan 31, 2018 at 03:22:49PM +0100, Kevin Wolf wrote: > > Am 31.01.2018 um 15:12 hat Stefan Hajnoczi geschrieben: > > > There should be a separate paragraph in docs/qemu-block-drivers.texi > > > explaining that share-rw=on can be used safely with format=raw if the > > > guests are configured to safely access a shared disk. It should also > > > mention that share-rw=on is unsafe for image formats. > > > > share-rw=on is a -device option and only about the guest, not about the > > backend. It is never unsafe if the guest can cope with external writers. > > It just doesn't prevent qemu from locking the image file for image > > formats. > > Thanks for the explanation. Documentation on share-rw=on|off would be > nice. > > Maybe something like: > > By default the guest has exclusive write access to its disk image. > This is enforced by QEMU via file locking. If the guest can safely > share the disk image with other writers the -device ...,share-rw=on > parameter can be used. This is only safe if the guest is running > software, such as a cluster file system, that coordinates disk > accesses to avoid corruption. > > Note that share-rw=on only declares the guest's ability to share the > disk. Some QEMU features, such as image file formats, require > exclusive write access to the disk image and this is unaffected by the > share-rw=on option. I will add these paragraphs to the "image locking" seciton in docs/qemu-block-drivers.texi. Fam
diff --git a/qemu-img.texi b/qemu-img.texi index 60a0e080c6..ec7e2f5d1e 100644 --- a/qemu-img.texi +++ b/qemu-img.texi @@ -86,6 +86,13 @@ exclusive with the @var{-O} parameters. It is currently required to also use the @var{-n} parameter to skip image creation. This restriction may be relaxed in a future release. +@item --force-share (-U) +If specified, @code{qemu-img} will open the image in shared mode, allowing +concurrent writers. For example, this can be used to get the image information +(with 'info' subcommand) when the image is used by a running guest. Note that +this could produce inconsistent results because of concurrent metadata changes, +etc. This option is only allowed when opening images in read-only mode. + @item --backing-chain will enumerate information about backing files in a disk image chain. Refer below for further description.