From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=36434 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PkGyY-00026M-0v for qemu-devel@nongnu.org; Tue, 01 Feb 2011 09:12:59 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PkGyW-0002no-LQ for qemu-devel@nongnu.org; Tue, 01 Feb 2011 09:12:58 -0500 Received: from mail-yx0-f173.google.com ([209.85.213.173]:57982) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PkGyW-0002nf-Gl for qemu-devel@nongnu.org; Tue, 01 Feb 2011 09:12:56 -0500 Received: by yxl31 with SMTP id 31so2764911yxl.4 for ; Tue, 01 Feb 2011 06:12:55 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <1296557928-30019-2-git-send-email-Jes.Sorensen@redhat.com> References: <1296557928-30019-1-git-send-email-Jes.Sorensen@redhat.com> <1296557928-30019-2-git-send-email-Jes.Sorensen@redhat.com> Date: Tue, 1 Feb 2011 14:12:55 +0000 Message-ID: Subject: Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw From: Stefan Hajnoczi Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jes.Sorensen@redhat.com Cc: agl@us.ibm.com, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com On Tue, Feb 1, 2011 at 10:58 AM, wrote: > From: Jes Sorensen > > Implement freeze/thaw support in the guest, allowing the host to > request the guest freezes all it's file systems before a live snapshot > is performed. > =A0- fsfreeze(): Walk the list of mounted local real file systems, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 and freeze them. > =A0- fsthaw(): =A0 Walk the list of previously frozen file systems and > =A0 =A0 =A0 =A0 =A0 =A0 =A0 thaw them. > =A0- fsstatus(): Return the current status of freeze/thaw. The host must > =A0 =A0 =A0 =A0 =A0 =A0 =A0 poll this function, in case fsfreeze() return= ed with a > =A0 =A0 =A0 =A0 =A0 =A0 =A0 timeout, to wait for the operation to finish. It is desirable to minimize the freeze time, which may interrupt or degrade the service that applications inside the VM can provide. Polling means we have to choose a fixed value (500 ms?) at which to check for freeze completion. In this example we could have up to 500 ms extra time spent in freeze because it completed right after we polled. Any thoughts on this? In terms of the fsfreeze(), fsthaw(), fsstatus() API, are you looking at Windows Volume Shadow Copy Services and does this API fit that model (I haven't looked at it in detail yet)? http://msdn.microsoft.com/en-us/library/bb968832(v=3Dvs.85).aspx > Signed-off-by: Jes Sorensen > --- > =A0virtagent-common.h | =A0 =A08 ++ > =A0virtagent-server.c | =A0196 ++++++++++++++++++++++++++++++++++++++++++= ++++++++++ > =A02 files changed, 204 insertions(+), 0 deletions(-) > > diff --git a/virtagent-common.h b/virtagent-common.h > index 5d8f5c1..220a4b6 100644 > --- a/virtagent-common.h > +++ b/virtagent-common.h > @@ -61,6 +61,14 @@ typedef struct VAContext { > =A0 =A0 const char *channel_path; > =A0} VAContext; > > +enum vs_fsfreeze_status { > + =A0 =A0FREEZE_ERROR =3D -1, > + =A0 =A0FREEZE_THAWED =3D 0, > + =A0 =A0FREEZE_INPROGRESS =3D 1, > + =A0 =A0FREEZE_FROZEN =3D 2, > + =A0 =A0FREEZE_THAWINPROGRESS =3D 3, > +}; > + > =A0enum va_job_status { > =A0 =A0 VA_JOB_STATUS_PENDING =3D 0, > =A0 =A0 VA_JOB_STATUS_OK, > diff --git a/virtagent-server.c b/virtagent-server.c > index 7bb35b2..cf2a3f0 100644 > --- a/virtagent-server.c > +++ b/virtagent-server.c > @@ -14,6 +14,13 @@ > =A0#include > =A0#include "qemu_socket.h" > =A0#include "virtagent-common.h" > +#include > +#include > +#include > +#include > +#include > +#include > +#include > > =A0static VAServerData *va_server_data; > =A0static bool va_enable_syslog =3D false; /* enable syslog'ing of RPCs *= / > @@ -217,6 +224,189 @@ static xmlrpc_value *va_hello(xmlrpc_env *env, > =A0 =A0 return result; > =A0} > > + > +/* > + * Walk the mount table and build a list of local file systems > + */ > + > +struct direntry { > + =A0 =A0char *dirname; > + =A0 =A0char *devtype; > + =A0 =A0struct direntry *next; > +}; > + > +static struct direntry *mount_list; > +static int fsfreeze_status; > + > +static int build_mount_list(void) > +{ > + =A0 =A0struct mntent *mnt; > + =A0 =A0struct direntry *entry; > + =A0 =A0struct direntry *next; > + =A0 =A0char const *mtab =3D MOUNTED; > + =A0 =A0FILE *fp; > + > + =A0 =A0fp =3D setmntent(mtab, "r"); > + =A0 =A0if (!fp) { > + =A0 =A0 =A0 fprintf(stderr, "unable to read mtab\n"); > + =A0 =A0 =A0 goto fail; > + =A0 =A0} > + > + =A0 =A0while ((mnt =3D getmntent(fp))) { > + =A0 =A0 =A0 /* > + =A0 =A0 =A0 =A0* An entry which device name doesn't start with a '/' is > + =A0 =A0 =A0 =A0* either a dummy file system or a network file system. > + =A0 =A0 =A0 =A0* Add special handling for smbfs and cifs as is done by > + =A0 =A0 =A0 =A0* coreutils as well. > + =A0 =A0 =A0 =A0*/ > + =A0 =A0 =A0 if ((mnt->mnt_fsname[0] !=3D '/') || > + =A0 =A0 =A0 =A0 =A0 (strcmp(mnt->mnt_type, "smbfs") =3D=3D 0) || > + =A0 =A0 =A0 =A0 =A0 (strcmp(mnt->mnt_type, "cifs") =3D=3D 0)) { > + =A0 =A0 =A0 =A0 =A0 continue; > + =A0 =A0 =A0 } > + > + =A0 =A0 =A0 entry =3D qemu_malloc(sizeof(struct direntry)); > + =A0 =A0 =A0 if (!entry) { > + =A0 =A0 =A0 =A0 =A0 goto fail; > + =A0 =A0 =A0 } qemu_malloc() never fails. > + =A0 =A0 =A0 entry->dirname =3D qemu_strdup(mnt->mnt_dir); > + =A0 =A0 =A0 entry->devtype =3D qemu_strdup(mnt->mnt_type); > + =A0 =A0 =A0 entry->next =3D mount_list; > + > + =A0 =A0 =A0 mount_list =3D entry; > + =A0 =A0} > + > + =A0 =A0endmntent(fp); > + > + =A0 =A0return 0; > + > +fail: > + =A0 =A0while(mount_list) { > + =A0 =A0 =A0 next =3D mount_list->next; > + =A0 =A0 =A0 qemu_free(mount_list->dirname); > + =A0 =A0 =A0 qemu_free(mount_list->devtype); > + =A0 =A0 =A0 qemu_free(mount_list); > + =A0 =A0 =A0 mount_list =3D next; > + =A0 =A0} > + > + =A0 =A0return -1; > +} > + > +/* > + * va_fsfreeze(): Walk list of mounted file systems in the guest, and > + * =A0 freeze the ones which are real local file systems. > + * rpc return values: Number of file systems frozen, -1 on error. > + */ > +static xmlrpc_value *va_fsfreeze(xmlrpc_env *env, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 xmlrpc_= value *params, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 void *u= ser_data) > +{ > + =A0 =A0xmlrpc_int32 ret =3D 0, i =3D 0; > + =A0 =A0xmlrpc_value *result; > + =A0 =A0struct direntry *entry; > + =A0 =A0int fd; > + =A0 =A0SLOG("va_fsfreeze()"); > + > + =A0 =A0if (fsfreeze_status =3D=3D FREEZE_FROZEN) { > + =A0 =A0 =A0 =A0ret =3D 0; > + =A0 =A0 =A0 =A0goto out; > + =A0 =A0} The only valid status here is FREEZE_THAWED? Perhaps we should test for that specifically. > + > + =A0 =A0ret =3D build_mount_list(); > + =A0 =A0if (ret < 0) { > + =A0 =A0 =A0 =A0goto out; > + =A0 =A0} > + > + =A0 =A0fsfreeze_status =3D FREEZE_INPROGRESS; > + > + =A0 =A0entry =3D mount_list; > + =A0 =A0while(entry) { > + =A0 =A0 =A0 =A0fd =3D qemu_open(entry->dirname, O_RDONLY); > + =A0 =A0 =A0 =A0if (fd =3D=3D -1) { > + =A0 =A0 =A0 =A0 =A0 =A0ret =3D errno; > + =A0 =A0 =A0 =A0 =A0 =A0goto error; > + =A0 =A0 =A0 =A0} > + =A0 =A0 =A0 =A0ret =3D ioctl(fd, FIFREEZE); If you close(fd) here then it won't leak or need extra code in the error pa= th. > + =A0 =A0 =A0 =A0if (ret < 0 && ret !=3D EOPNOTSUPP) { > + =A0 =A0 =A0 =A0 =A0 =A0goto error; > + =A0 =A0 =A0 =A0} > + > + =A0 =A0 =A0 =A0close(fd); > + =A0 =A0 =A0 =A0entry =3D entry->next; > + =A0 =A0 =A0 =A0i++; > + =A0 =A0} > + > + =A0 =A0fsfreeze_status =3D FREEZE_FROZEN; > + =A0 =A0ret =3D i; > +out: > + =A0 =A0result =3D xmlrpc_build_value(env, "i", ret); > + =A0 =A0return result; > +error: > + =A0 =A0if (i > 0) { > + =A0 =A0 =A0 =A0fsfreeze_status =3D FREEZE_ERROR; > + =A0 =A0} > + =A0 =A0goto out; > +} > + > +/* > + * va_fsthaw(): Walk list of frozen file systems in the guest, and > + * =A0 thaw them. > + * rpc return values: Number of file systems thawed on success, -1 on er= ror. > + */ > +static xmlrpc_value *va_fsthaw(xmlrpc_env *env, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 xmlrpc_valu= e *params, > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 void *user_= data) > +{ > + =A0 =A0xmlrpc_int32 ret; > + =A0 =A0xmlrpc_value *result; > + =A0 =A0struct direntry *entry; > + =A0 =A0int fd, i =3D 0; > + =A0 =A0SLOG("va_fsthaw()"); > + > + =A0 =A0if (fsfreeze_status =3D=3D FREEZE_THAWED) { > + =A0 =A0 =A0 =A0ret =3D 0; > + =A0 =A0 =A0 =A0goto out; > + =A0 =A0} A stricter check would be status FREEZE_FROZEN. > + > + =A0 =A0while((entry =3D mount_list)) { > + =A0 =A0 =A0 =A0fd =3D qemu_open(entry->dirname, O_RDONLY); > + =A0 =A0 =A0 =A0if (fd =3D=3D -1) { > + =A0 =A0 =A0 =A0 =A0 =A0ret =3D -1; > + =A0 =A0 =A0 =A0 =A0 =A0goto out; > + =A0 =A0 =A0 =A0} > + =A0 =A0 =A0 =A0ret =3D ioctl(fd, FITHAW); Same thing about close(fd) here. Stefan