| Message ID | 1485876994-14555-1-git-send-email-pl@kamp.de |
|---|---|
| State | New |
| Headers | show |
On 31/01/2017 07:36, Peter Lieven wrote: > the current implementation fails if we try to freeze an > already frozen filesystem. This can happen if a filesystem > is mounted more than once (e.g. with a bind mount). > > Suggested-by: Christian Theune <ct@flyingcircus.io> > Cc: qemu-stable@nongnu.org > Signed-off-by: Peter Lieven <pl@kamp.de> What happens when you thaw? Paolo
Hi, to clarify: I guess you ask about what happens when you thaw with the same condition: a filesystem that is mounted multiple times. When I created the patch initially I looked at the thaw code and found it to already expect that thaws may happen outside its control and that the error handling is fine. The code definitely does not have an early exit upon encountering an issue with thawing/double thawing. Christian > On 2 Feb 2017, at 22:41, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > On 31/01/2017 07:36, Peter Lieven wrote: >> the current implementation fails if we try to freeze an >> already frozen filesystem. This can happen if a filesystem >> is mounted more than once (e.g. with a bind mount). >> >> Suggested-by: Christian Theune <ct@flyingcircus.io> >> Cc: qemu-stable@nongnu.org >> Signed-off-by: Peter Lieven <pl@kamp.de> > > What happens when you thaw? > > Paolo -- Christian Theune · ct@flyingcircus.io · +49 345 219401 0 Flying Circus Internet Operations GmbH · http://flyingcircus.io Forsterstraße 29 · 06112 Halle (Saale) · Deutschland HR Stendal HRB 21169 · Geschäftsführer: Christian. Theune, Christian. Zagrodnick
Am 02.02.2017 um 22:41 schrieb Paolo Bonzini: > > On 31/01/2017 07:36, Peter Lieven wrote: >> the current implementation fails if we try to freeze an >> already frozen filesystem. This can happen if a filesystem >> is mounted more than once (e.g. with a bind mount). >> >> Suggested-by: Christian Theune <ct@flyingcircus.io> >> Cc: qemu-stable@nongnu.org >> Signed-off-by: Peter Lieven <pl@kamp.de> > What happens when you thaw? > > Paolo If you try to THAW an unfrozen FS you get EINVAL. The current code thaws until an error is returned. So it should work as is. If you feel uncomfortable with the EBUSY approach. The other idea would be to track all devices which have been successfully frozen and skip consecutive tries to freeze them. Peter
Hi, > On 3 Feb 2017, at 09:20, Peter Lieven <pl@kamp.de> wrote: > > If you try to THAW an unfrozen FS you get EINVAL. > > The current code thaws until an error is returned. > > > So it should work as is. > > > If you feel uncomfortable with the EBUSY approach. The other idea would > > be to track all devices which have been successfully frozen and skip consecutive > > tries to freeze them. Which will be subject to race conditions as other processes may call freeze/thaw on any device. Those are completely out of our control. Christian -- Christian Theune · ct@flyingcircus.io · +49 345 219401 0 Flying Circus Internet Operations GmbH · http://flyingcircus.io Forsterstraße 29 · 06112 Halle (Saale) · Deutschland HR Stendal HRB 21169 · Geschäftsführer: Christian. Theune, Christian. Zagrodnick
Am 03.02.2017 um 09:22 schrieb Christian Theune: > Hi, > >> On 3 Feb 2017, at 09:20, Peter Lieven <pl@kamp.de <mailto:pl@kamp.de>> wrote: >> >> If you try to THAW an unfrozen FS you get EINVAL. >> >> The current code thaws until an error is returned. >> >> >> So it should work as is. >> >> >> If you feel uncomfortable with the EBUSY approach. The other idea would >> >> be to track all devices which have been successfully frozen and skip consecutive >> >> tries to freeze them. > > Which will be subject to race conditions as other processes may call freeze/thaw on any device. Those are completely out of our control. That will always be out of control. If we freeze the FS and another process thaws it, we snapshot an unfrozen fs and believe it is frozen. I think the EBUSY approach is fine. If I look at the documentation of the thaw in the code it seems that the behaviour of the linux kernel changed. It seems that it was possible to call FIFREEZE multiple times on an FS and this required multiple FITHAWs to unfreeze it. Peter
On 03/02/2017 00:20, Peter Lieven wrote: > Am 02.02.2017 um 22:41 schrieb Paolo Bonzini: >> >> On 31/01/2017 07:36, Peter Lieven wrote: >>> the current implementation fails if we try to freeze an >>> already frozen filesystem. This can happen if a filesystem >>> is mounted more than once (e.g. with a bind mount). >>> >>> Suggested-by: Christian Theune <ct@flyingcircus.io> >>> Cc: qemu-stable@nongnu.org >>> Signed-off-by: Peter Lieven <pl@kamp.de> >> What happens when you thaw? >> >> Paolo > > If you try to THAW an unfrozen FS you get EINVAL. > The current code thaws until an error is returned. > > So it should work as is. > > If you feel uncomfortable with the EBUSY approach. The other idea would > be to track all devices which have been successfully frozen and skip consecutive > tries to freeze them. No, just asking. Reeviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Am 03.02.2017 um 19:31 schrieb Paolo Bonzini: > > On 03/02/2017 00:20, Peter Lieven wrote: >> Am 02.02.2017 um 22:41 schrieb Paolo Bonzini: >>> On 31/01/2017 07:36, Peter Lieven wrote: >>>> the current implementation fails if we try to freeze an >>>> already frozen filesystem. This can happen if a filesystem >>>> is mounted more than once (e.g. with a bind mount). >>>> >>>> Suggested-by: Christian Theune <ct@flyingcircus.io> >>>> Cc: qemu-stable@nongnu.org >>>> Signed-off-by: Peter Lieven <pl@kamp.de> >>> What happens when you thaw? >>> >>> Paolo >> If you try to THAW an unfrozen FS you get EINVAL. >> The current code thaws until an error is returned. >> >> So it should work as is. >> >> If you feel uncomfortable with the EBUSY approach. The other idea would >> be to track all devices which have been successfully frozen and skip consecutive >> tries to freeze them. > No, just asking. > > Reeviewed-by: Paolo Bonzini <pbonzini@redhat.com> Ping, Michael can you pick this up please. Thanks, Peter
diff --git a/qga/commands-posix.c b/qga/commands-posix.c index ea37c09..73d93eb 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -1243,6 +1243,9 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints, * filesystems may not implement fsfreeze for less obvious reasons. * these will report EOPNOTSUPP. we simply ignore these when tallying * the number of frozen filesystems. + * if a filesystem is mounted more than once (aka bind mount) a + * consecutive attempt to freeze an already frozen filesystem will + * return EBUSY. * * any other error means a failure to freeze a filesystem we * expect to be freezable, so return an error in those cases @@ -1250,7 +1253,7 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints, */ ret = ioctl(fd, FIFREEZE); if (ret == -1) { - if (errno != EOPNOTSUPP) { + if (errno != EOPNOTSUPP && errno != EBUSY) { error_setg_errno(errp, errno, "failed to freeze %s", mount->dirname); close(fd);
the current implementation fails if we try to freeze an already frozen filesystem. This can happen if a filesystem is mounted more than once (e.g. with a bind mount). Suggested-by: Christian Theune <ct@flyingcircus.io> Cc: qemu-stable@nongnu.org Signed-off-by: Peter Lieven <pl@kamp.de> --- qga/commands-posix.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)