* [Qemu-devel] [PATCH v2 0/2] virtagent - fsfreeze support @ 2011-02-02 8:42 Jes.Sorensen 2011-02-02 8:42 ` [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw Jes.Sorensen ` (2 more replies) 0 siblings, 3 replies; 22+ messages in thread From: Jes.Sorensen @ 2011-02-02 8:42 UTC (permalink / raw) To: qemu-devel; +Cc: lcapitulino, mdroth, stefanha, agl From: Jes Sorensen <Jes.Sorensen@redhat.com> Hi This is a first attempt to add fsfreeze support to virtagent. The idea is for the guest agent to walk the list of locally mounted file systems in the guest, and issuing an ioctl to freeze them. The host can then do a live snapshot of the guest, obtaining stable file systems. After the snapshot, the host then calls the thaw function in virtagent, which goes through the list of previously frozen file systems and unfreezes them. The list walking ignores remote file systems such as NFS and CIFS as well as all pseudo file systems. The guest agent code is in the first patch, and host agent code is in the second patch. For now there is only human monitor support, but it should be pretty straight forward to add QMP support as well. Comments and suggestions welcome! v2 of the patch addresses the issues pointed out by Stefan and Michael. Cheers, Jes Jes Sorensen (2): Add virtagent file system freeze/thaw Add monitor commands for fsfreeze support hmp-commands.hx | 48 +++++++++++ virtagent-common.h | 9 ++ virtagent-server.c | 190 ++++++++++++++++++++++++++++++++++++++++++ virtagent.c | 235 ++++++++++++++++++++++++++++++++++++++++++++++++++++ virtagent.h | 9 ++ 5 files changed, 491 insertions(+), 0 deletions(-) -- 1.7.3.5 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw 2011-02-02 8:42 [Qemu-devel] [PATCH v2 0/2] virtagent - fsfreeze support Jes.Sorensen @ 2011-02-02 8:42 ` Jes.Sorensen 2011-02-03 18:11 ` [Qemu-devel] " Michael Roth 2011-02-02 8:42 ` [Qemu-devel] [PATCH 2/2] Add monitor commands for fsfreeze support Jes.Sorensen 2011-02-03 18:15 ` [Qemu-devel] Re: [PATCH v2 0/2] virtagent - " Michael Roth 2 siblings, 1 reply; 22+ messages in thread From: Jes.Sorensen @ 2011-02-02 8:42 UTC (permalink / raw) To: qemu-devel; +Cc: lcapitulino, mdroth, stefanha, agl From: Jes Sorensen <Jes.Sorensen@redhat.com> 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. - fsfreeze(): Walk the list of mounted local real file systems, and freeze them. - fsthaw(): Walk the list of previously frozen file systems and thaw them. - fsstatus(): Return the current status of freeze/thaw. The host must poll this function, in case fsfreeze() returned with a timeout, to wait for the operation to finish. Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com> --- virtagent-common.h | 8 ++ virtagent-server.c | 190 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 198 insertions(+), 0 deletions(-) diff --git a/virtagent-common.h b/virtagent-common.h index 5d8f5c1..7c6d9ef 100644 --- a/virtagent-common.h +++ b/virtagent-common.h @@ -61,6 +61,14 @@ typedef struct VAContext { const char *channel_path; } VAContext; +enum va_fsfreeze_status { + FREEZE_ERROR = -1, + FREEZE_THAWED = 0, + FREEZE_INPROGRESS = 1, + FREEZE_FROZEN = 2, + FREEZE_THAWINPROGRESS = 3, +}; + enum va_job_status { VA_JOB_STATUS_PENDING = 0, VA_JOB_STATUS_OK, diff --git a/virtagent-server.c b/virtagent-server.c index 7bb35b2..ffbe163 100644 --- a/virtagent-server.c +++ b/virtagent-server.c @@ -14,6 +14,10 @@ #include <syslog.h> #include "qemu_socket.h" #include "virtagent-common.h" +#include <mntent.h> +#include <sys/types.h> +#include <sys/ioctl.h> +#include <linux/fs.h> static VAServerData *va_server_data; static bool va_enable_syslog = false; /* enable syslog'ing of RPCs */ @@ -217,6 +221,186 @@ static xmlrpc_value *va_hello(xmlrpc_env *env, return result; } + +/* + * Walk the mount table and build a list of local file systems + */ + +struct direntry { + char *dirname; + char *devtype; + struct direntry *next; +}; + +static struct direntry *va_mount_list; +static int va_fsfreeze_status; + +static int build_mount_list(void) +{ + struct mntent *mnt; + struct direntry *entry; + struct direntry *next; + char const *mtab = MOUNTED; + FILE *fp; + + fp = setmntent(mtab, "r"); + if (!fp) { + fprintf(stderr, "unable to read mtab\n"); + goto fail; + } + + while ((mnt = getmntent(fp))) { + /* + * An entry which device name doesn't start with a '/' is + * either a dummy file system or a network file system. + * Add special handling for smbfs and cifs as is done by + * coreutils as well. + */ + if ((mnt->mnt_fsname[0] != '/') || + (strcmp(mnt->mnt_type, "smbfs") == 0) || + (strcmp(mnt->mnt_type, "cifs") == 0)) { + continue; + } + + entry = qemu_malloc(sizeof(struct direntry)); + entry->dirname = qemu_strdup(mnt->mnt_dir); + entry->devtype = qemu_strdup(mnt->mnt_type); + entry->next = va_mount_list; + + va_mount_list = entry; + } + + endmntent(fp); + + return 0; + +fail: + while(va_mount_list) { + next = va_mount_list->next; + qemu_free(va_mount_list->dirname); + qemu_free(va_mount_list->devtype); + qemu_free(va_mount_list); + va_mount_list = next; + } + + return -1; +} + +/* + * va_fsfreeze(): Walk list of mounted file systems in the guest, and + * 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, + xmlrpc_value *params, + void *user_data) +{ + xmlrpc_int32 ret = 0, i = 0; + xmlrpc_value *result; + struct direntry *entry; + int fd; + SLOG("va_fsfreeze()"); + + if (va_fsfreeze_status != FREEZE_THAWED) { + ret = 0; + goto out; + } + + ret = build_mount_list(); + if (ret < 0) { + goto out; + } + + va_fsfreeze_status = FREEZE_INPROGRESS; + + entry = va_mount_list; + while(entry) { + fd = qemu_open(entry->dirname, O_RDONLY); + if (fd == -1) { + ret = errno; + goto error; + } + ret = ioctl(fd, FIFREEZE); + close(fd); + if (ret < 0 && ret != EOPNOTSUPP) { + goto error; + } + + entry = entry->next; + i++; + } + + va_fsfreeze_status = FREEZE_FROZEN; + ret = i; +out: + result = xmlrpc_build_value(env, "i", ret); + return result; +error: + if (i > 0) { + va_fsfreeze_status = FREEZE_ERROR; + } + goto out; +} + +/* + * va_fsthaw(): Walk list of frozen file systems in the guest, and + * thaw them. + * rpc return values: Number of file systems thawed on success, -1 on error. + */ +static xmlrpc_value *va_fsthaw(xmlrpc_env *env, + xmlrpc_value *params, + void *user_data) +{ + xmlrpc_int32 ret; + xmlrpc_value *result; + struct direntry *entry; + int fd, i = 0; + SLOG("va_fsthaw()"); + + if (va_fsfreeze_status != FREEZE_FROZEN) { + ret = 0; + goto out; + } + + while((entry = va_mount_list)) { + fd = qemu_open(entry->dirname, O_RDONLY); + if (fd == -1) { + ret = -1; + goto out; + } + ret = ioctl(fd, FITHAW); + close(fd); + if (ret < 0 && ret != EOPNOTSUPP) { + ret = -1; + goto out; + } + + va_mount_list = entry->next; + qemu_free(entry->dirname); + qemu_free(entry->devtype); + qemu_free(entry); + i++; + } + + va_fsfreeze_status = FREEZE_THAWED; + ret = i; +out: + result = xmlrpc_build_value(env, "i", ret); + return result; +} + +/* va_fsstatus(): Return status of freeze/thaw + * rpc return values: fsfreeze_status + */ +static xmlrpc_value *va_fsstatus(xmlrpc_env *env, + xmlrpc_value *params, + void *user_data) +{ + xmlrpc_value *result = xmlrpc_build_value(env, "i", va_fsfreeze_status); + SLOG("va_fsstatus()"); + return result; +} + typedef struct RPCFunction { xmlrpc_value *(*func)(xmlrpc_env *env, xmlrpc_value *param, void *unused); const char *func_name; @@ -237,6 +421,12 @@ static RPCFunction guest_functions[] = { .func_name = "va.ping" }, { .func = va_capabilities, .func_name = "va.capabilities" }, + { .func = va_fsfreeze, + .func_name = "va.fsfreeze" }, + { .func = va_fsthaw, + .func_name = "va.fsthaw" }, + { .func = va_fsstatus, + .func_name = "va.fsstatus" }, { NULL, NULL } }; static RPCFunction host_functions[] = { -- 1.7.3.5 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] Re: [PATCH 1/2] Add virtagent file system freeze/thaw 2011-02-02 8:42 ` [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw Jes.Sorensen @ 2011-02-03 18:11 ` Michael Roth 2011-02-04 10:54 ` Jes Sorensen 0 siblings, 1 reply; 22+ messages in thread From: Michael Roth @ 2011-02-03 18:11 UTC (permalink / raw) To: Jes.Sorensen; +Cc: lcapitulino, qemu-devel, stefanha, agl On 02/02/2011 02:42 AM, Jes.Sorensen@redhat.com wrote: > From: Jes Sorensen<Jes.Sorensen@redhat.com> > > 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. > - fsfreeze(): Walk the list of mounted local real file systems, > and freeze them. > - fsthaw(): Walk the list of previously frozen file systems and > thaw them. > - fsstatus(): Return the current status of freeze/thaw. The host must > poll this function, in case fsfreeze() returned with a > timeout, to wait for the operation to finish. > > Signed-off-by: Jes Sorensen<Jes.Sorensen@redhat.com> > --- > virtagent-common.h | 8 ++ > virtagent-server.c | 190 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 198 insertions(+), 0 deletions(-) > > diff --git a/virtagent-common.h b/virtagent-common.h > index 5d8f5c1..7c6d9ef 100644 > --- a/virtagent-common.h > +++ b/virtagent-common.h > @@ -61,6 +61,14 @@ typedef struct VAContext { > const char *channel_path; > } VAContext; > > +enum va_fsfreeze_status { > + FREEZE_ERROR = -1, > + FREEZE_THAWED = 0, > + FREEZE_INPROGRESS = 1, > + FREEZE_FROZEN = 2, > + FREEZE_THAWINPROGRESS = 3, > +}; > + > enum va_job_status { > VA_JOB_STATUS_PENDING = 0, > VA_JOB_STATUS_OK, > diff --git a/virtagent-server.c b/virtagent-server.c > index 7bb35b2..ffbe163 100644 > --- a/virtagent-server.c > +++ b/virtagent-server.c > @@ -14,6 +14,10 @@ > #include<syslog.h> > #include "qemu_socket.h" > #include "virtagent-common.h" > +#include<mntent.h> > +#include<sys/types.h> > +#include<sys/ioctl.h> > +#include<linux/fs.h> > > static VAServerData *va_server_data; > static bool va_enable_syslog = false; /* enable syslog'ing of RPCs */ > @@ -217,6 +221,186 @@ static xmlrpc_value *va_hello(xmlrpc_env *env, > return result; > } > > + > +/* > + * Walk the mount table and build a list of local file systems > + */ > + > +struct direntry { > + char *dirname; > + char *devtype; > + struct direntry *next; > +}; > + > +static struct direntry *va_mount_list; > +static int va_fsfreeze_status; And what I meant in the last RFC about using "objects" was to encapsulate global state information for a particular group of commands in single data type/variable. We're gonna end up with a similar set of variables for stateful RPCs like copyfile and potentially a few for things like spice. So to avoid having things get too cluttered up I'd prefer something like, in this particular case: typedef struct VAFSFreezeState { struct direntry *mount_list; int status; } VAFSFeezeState; static VAFSFreezeState va_fsfreeze_state; > + > +static int build_mount_list(void) > +{ > + struct mntent *mnt; > + struct direntry *entry; > + struct direntry *next; > + char const *mtab = MOUNTED; > + FILE *fp; > + > + fp = setmntent(mtab, "r"); > + if (!fp) { > + fprintf(stderr, "unable to read mtab\n"); > + goto fail; > + } You have tabs instead of spaces here > + > + while ((mnt = getmntent(fp))) { > + /* > + * An entry which device name doesn't start with a '/' is > + * either a dummy file system or a network file system. > + * Add special handling for smbfs and cifs as is done by > + * coreutils as well. > + */ > + if ((mnt->mnt_fsname[0] != '/') || > + (strcmp(mnt->mnt_type, "smbfs") == 0) || > + (strcmp(mnt->mnt_type, "cifs") == 0)) { > + continue; > + } > + > + entry = qemu_malloc(sizeof(struct direntry)); > + entry->dirname = qemu_strdup(mnt->mnt_dir); > + entry->devtype = qemu_strdup(mnt->mnt_type); > + entry->next = va_mount_list; > + > + va_mount_list = entry; Here too > + } > + > + endmntent(fp); > + > + return 0; > + > +fail: > + while(va_mount_list) { > + next = va_mount_list->next; Here too. Might be some other spots but you get the point :) > + qemu_free(va_mount_list->dirname); > + qemu_free(va_mount_list->devtype); > + qemu_free(va_mount_list); > + va_mount_list = next; > + } > + > + return -1; > +} > + > +/* > + * va_fsfreeze(): Walk list of mounted file systems in the guest, and > + * 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, > + xmlrpc_value *params, > + void *user_data) > +{ > + xmlrpc_int32 ret = 0, i = 0; > + xmlrpc_value *result; > + struct direntry *entry; > + int fd; > + SLOG("va_fsfreeze()"); > + > + if (va_fsfreeze_status != FREEZE_THAWED) { > + ret = 0; > + goto out; > + } > + > + ret = build_mount_list(); > + if (ret< 0) { > + goto out; > + } > + > + va_fsfreeze_status = FREEZE_INPROGRESS; > + > + entry = va_mount_list; > + while(entry) { > + fd = qemu_open(entry->dirname, O_RDONLY); > + if (fd == -1) { > + ret = errno; > + goto error; > + } > + ret = ioctl(fd, FIFREEZE); > + close(fd); > + if (ret< 0&& ret != EOPNOTSUPP) { > + goto error; > + } > + > + entry = entry->next; > + i++; > + } > + > + va_fsfreeze_status = FREEZE_FROZEN; > + ret = i; > +out: > + result = xmlrpc_build_value(env, "i", ret); > + return result; > +error: > + if (i> 0) { > + va_fsfreeze_status = FREEZE_ERROR; > + } > + goto out; > +} > + > +/* > + * va_fsthaw(): Walk list of frozen file systems in the guest, and > + * thaw them. > + * rpc return values: Number of file systems thawed on success, -1 on error. > + */ > +static xmlrpc_value *va_fsthaw(xmlrpc_env *env, > + xmlrpc_value *params, > + void *user_data) > +{ > + xmlrpc_int32 ret; > + xmlrpc_value *result; > + struct direntry *entry; > + int fd, i = 0; > + SLOG("va_fsthaw()"); > + > + if (va_fsfreeze_status != FREEZE_FROZEN) { > + ret = 0; > + goto out; > + } > + > + while((entry = va_mount_list)) { > + fd = qemu_open(entry->dirname, O_RDONLY); > + if (fd == -1) { > + ret = -1; > + goto out; > + } > + ret = ioctl(fd, FITHAW); > + close(fd); > + if (ret< 0&& ret != EOPNOTSUPP) { > + ret = -1; > + goto out; > + } > + > + va_mount_list = entry->next; > + qemu_free(entry->dirname); > + qemu_free(entry->devtype); > + qemu_free(entry); > + i++; > + } > + > + va_fsfreeze_status = FREEZE_THAWED; > + ret = i; > +out: > + result = xmlrpc_build_value(env, "i", ret); > + return result; > +} > + > +/* va_fsstatus(): Return status of freeze/thaw > + * rpc return values: fsfreeze_status > + */ > +static xmlrpc_value *va_fsstatus(xmlrpc_env *env, > + xmlrpc_value *params, > + void *user_data) > +{ > + xmlrpc_value *result = xmlrpc_build_value(env, "i", va_fsfreeze_status); > + SLOG("va_fsstatus()"); > + return result; > +} > + > typedef struct RPCFunction { > xmlrpc_value *(*func)(xmlrpc_env *env, xmlrpc_value *param, void *unused); > const char *func_name; > @@ -237,6 +421,12 @@ static RPCFunction guest_functions[] = { > .func_name = "va.ping" }, > { .func = va_capabilities, > .func_name = "va.capabilities" }, > + { .func = va_fsfreeze, > + .func_name = "va.fsfreeze" }, > + { .func = va_fsthaw, > + .func_name = "va.fsthaw" }, > + { .func = va_fsstatus, > + .func_name = "va.fsstatus" }, > { NULL, NULL } > }; > static RPCFunction host_functions[] = { ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] Re: [PATCH 1/2] Add virtagent file system freeze/thaw 2011-02-03 18:11 ` [Qemu-devel] " Michael Roth @ 2011-02-04 10:54 ` Jes Sorensen 0 siblings, 0 replies; 22+ messages in thread From: Jes Sorensen @ 2011-02-04 10:54 UTC (permalink / raw) To: Michael Roth; +Cc: lcapitulino, qemu-devel, stefanha, agl On 02/03/11 19:11, Michael Roth wrote: >> @@ -217,6 +221,186 @@ static xmlrpc_value *va_hello(xmlrpc_env *env, >> return result; >> } >> >> + >> +/* >> + * Walk the mount table and build a list of local file systems >> + */ >> + >> +struct direntry { >> + char *dirname; >> + char *devtype; >> + struct direntry *next; >> +}; >> + >> +static struct direntry *va_mount_list; >> +static int va_fsfreeze_status; > > And what I meant in the last RFC about using "objects" was to > encapsulate global state information for a particular group of commands > in single data type/variable. We're gonna end up with a similar set of > variables for stateful RPCs like copyfile and potentially a few for > things like spice. So to avoid having things get too cluttered up I'd > prefer something like, in this particular case: > > typedef struct VAFSFreezeState { > struct direntry *mount_list; > int status; > } VAFSFeezeState; > > static VAFSFreezeState va_fsfreeze_state; Ok, I got rid of the tabs (damn I thought I had caught them all), and added a struct to keep the freeze state. I didn't add any typedef grossness though. v3 coming up. Cheers, Jes ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 2/2] Add monitor commands for fsfreeze support 2011-02-02 8:42 [Qemu-devel] [PATCH v2 0/2] virtagent - fsfreeze support Jes.Sorensen 2011-02-02 8:42 ` [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw Jes.Sorensen @ 2011-02-02 8:42 ` Jes.Sorensen 2011-02-03 18:15 ` [Qemu-devel] Re: [PATCH v2 0/2] virtagent - " Michael Roth 2 siblings, 0 replies; 22+ messages in thread From: Jes.Sorensen @ 2011-02-02 8:42 UTC (permalink / raw) To: qemu-devel; +Cc: lcapitulino, mdroth, stefanha, agl From: Jes Sorensen <Jes.Sorensen@redhat.com> This patch adds the following monitor commands: agent_fsfreeze: - Freezes all local file systems in the guest. Command will print the number of file systems that were frozen. agent_fsthaw: - Thaws all local file systems in the guest. Command will print the number of file systems that were thawed. agent_fsstatus: - Prints the current status of file systems in the guest: Thawed, frozen, thaw in progress, freeze in progress, error. Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com> --- hmp-commands.hx | 48 +++++++++++ virtagent-common.h | 1 + virtagent.c | 235 ++++++++++++++++++++++++++++++++++++++++++++++++++++ virtagent.h | 9 ++ 4 files changed, 293 insertions(+), 0 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index 9c7ac0b..f4150da 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1310,6 +1310,54 @@ STEXI Fetch and re-negotiate guest agent capabilties ETEXI + { + .name = "agent_fsfreeze", + .args_type = "", + .params = "", + .help = "Freeze all local file systems mounted in the guest", + .user_print = do_agent_fsfreeze_print, + .mhandler.cmd_async = do_agent_fsfreeze, + .flags = MONITOR_CMD_ASYNC, + }, + +STEXI +@item agent_fsfreeze +@findex agent_fsfreeze +Freeze all local mounted file systems in guest +ETEXI + + { + .name = "agent_fsthaw", + .args_type = "", + .params = "", + .help = "Thaw all local file systems mounted in the guest", + .user_print = do_agent_fsthaw_print, + .mhandler.cmd_async = do_agent_fsthaw, + .flags = MONITOR_CMD_ASYNC, + }, + +STEXI +@item agent_fsthaw +@findex agent_fsthaw +Thaw all local mounted file systems in guest +ETEXI + + { + .name = "agent_fsstatus", + .args_type = "", + .params = "", + .help = "Display status of file system freeze progress in guest", + .user_print = do_agent_fsstatus_print, + .mhandler.cmd_async = do_agent_fsstatus, + .flags = MONITOR_CMD_ASYNC, + }, + +STEXI +@item agent_fsstatus +@findex agent_fsstatus +Get status of file system freeze in guest +ETEXI + STEXI @end table ETEXI diff --git a/virtagent-common.h b/virtagent-common.h index 7c6d9ef..ff7bf23 100644 --- a/virtagent-common.h +++ b/virtagent-common.h @@ -24,6 +24,7 @@ #include "monitor.h" #include "virtagent-server.h" #include "virtagent.h" +#include "qint.h" #define DEBUG_VA diff --git a/virtagent.c b/virtagent.c index b5e7944..4277802 100644 --- a/virtagent.c +++ b/virtagent.c @@ -640,3 +640,238 @@ int va_send_hello(void) xmlrpc_DECREF(params); return ret; } + +void do_agent_fsfreeze_print(Monitor *mon, const QObject *data) +{ + TRACE("called"); + + monitor_printf(mon, "File systems frozen: %" PRId64 "\n", + qint_get_int((qobject_to_qint(data)))); +} + +static void do_agent_fsfreeze_cb(const char *resp_data, + size_t resp_data_len, + MonitorCompletion *mon_cb, + void *mon_data) +{ + xmlrpc_value *resp = NULL; + xmlrpc_env env; + xmlrpc_int32 retval = 0; + QInt *qint; + + TRACE("called"); + + if (resp_data == NULL) { + LOG("error handling RPC request"); + return; + } + + xmlrpc_env_init(&env); + resp = xmlrpc_parse_response(&env, resp_data, resp_data_len); + if (va_rpc_has_error(&env)) { + LOG("error parsing RPC response"); + return; + } + + xmlrpc_parse_value(&env, resp, "i", &retval); + if (va_rpc_has_error(&env)) { + retval = -1; + goto out; + } + +out: + qint = qint_from_int(retval); + xmlrpc_DECREF(resp); + if (mon_cb) { + mon_cb(mon_data, QOBJECT(qint)); + } + qobject_decref(QOBJECT(qint)); +} + +int do_agent_fsfreeze(Monitor *mon, const QDict *mon_params, + MonitorCompletion cb, void *opaque) +{ + xmlrpc_env env; + xmlrpc_value *params; + int ret; + + TRACE("called"); + + xmlrpc_env_init(&env); + params = xmlrpc_build_value(&env, "()"); + if (va_rpc_has_error(&env)) { + return -1; + } + + ret = va_do_rpc(&env, "va.fsfreeze", params, do_agent_fsfreeze_cb, + cb, opaque); + if (ret) { + qerror_report(QERR_VA_FAILED, ret, strerror(ret)); + } + xmlrpc_DECREF(params); + return ret; +} + +void do_agent_fsthaw_print(Monitor *mon, const QObject *data) +{ + TRACE("called"); + + monitor_printf(mon, "File systems thawed: %" PRId64 "\n", + qint_get_int((qobject_to_qint(data)))); +} + +static void do_agent_fsthaw_cb(const char *resp_data, + size_t resp_data_len, + MonitorCompletion *mon_cb, + void *mon_data) +{ + xmlrpc_value *resp = NULL; + xmlrpc_env env; + xmlrpc_int32 retval = 0; + QInt *qint; + + TRACE("called"); + + if (resp_data == NULL) { + LOG("error handling RPC request"); + return; + } + + xmlrpc_env_init(&env); + resp = xmlrpc_parse_response(&env, resp_data, resp_data_len); + if (va_rpc_has_error(&env)) { + LOG("error parsing RPC response"); + return; + } + + xmlrpc_parse_value(&env, resp, "i", &retval); + if (va_rpc_has_error(&env)) { + retval = -1; + goto out; + } + +out: + qint = qint_from_int(retval); + xmlrpc_DECREF(resp); + if (mon_cb) { + mon_cb(mon_data, QOBJECT(qint)); + } + qobject_decref(QOBJECT(qint)); +} + +int do_agent_fsthaw(Monitor *mon, const QDict *mon_params, + MonitorCompletion cb, void *opaque) +{ + xmlrpc_env env; + xmlrpc_value *params; + int ret; + + TRACE("called"); + + xmlrpc_env_init(&env); + params = xmlrpc_build_value(&env, "()"); + if (va_rpc_has_error(&env)) { + return -1; + } + + ret = va_do_rpc(&env, "va.fsthaw", params, do_agent_fsthaw_cb, cb, opaque); + if (ret) { + qerror_report(QERR_VA_FAILED, ret, strerror(ret)); + } + xmlrpc_DECREF(params); + return ret; +} + +void do_agent_fsstatus_print(Monitor *mon, const QObject *data) +{ + int64_t fsstatus; + TRACE("called"); + + fsstatus = qint_get_int((qobject_to_qint(data))); + + monitor_printf(mon, "File systems freeze status: "); + switch(fsstatus) { + case FREEZE_THAWED: + monitor_printf(mon, "Thawed"); + break; + case FREEZE_INPROGRESS: + monitor_printf(mon, "Freeze in progress"); + break; + case FREEZE_THAWINPROGRESS: + monitor_printf(mon, "Thaw in progress"); + break; + case FREEZE_FROZEN: + monitor_printf(mon, "Frozen"); + break; + case FREEZE_ERROR: + monitor_printf(mon, "Error"); + break; + default: + monitor_printf(mon, "unknown"); + } + + monitor_printf(mon, "\n"); +} + +static void do_agent_fsstatus_cb(const char *resp_data, + size_t resp_data_len, + MonitorCompletion *mon_cb, + void *mon_data) +{ + xmlrpc_value *resp = NULL; + xmlrpc_env env; + xmlrpc_int32 retval = 0; + QInt *qint; + + TRACE("called"); + + if (resp_data == NULL) { + LOG("error handling RPC request"); + return; + } + + xmlrpc_env_init(&env); + resp = xmlrpc_parse_response(&env, resp_data, resp_data_len); + if (va_rpc_has_error(&env)) { + LOG("error parsing RPC response"); + return; + } + + xmlrpc_parse_value(&env, resp, "i", &retval); + if (va_rpc_has_error(&env)) { + retval = -1; + goto out; + } + +out: + qint = qint_from_int(retval); + xmlrpc_DECREF(resp); + if (mon_cb) { + mon_cb(mon_data, QOBJECT(qint)); + } + qobject_decref(QOBJECT(qint)); +} + +int do_agent_fsstatus(Monitor *mon, const QDict *mon_params, + MonitorCompletion cb, void *opaque) +{ + xmlrpc_env env; + xmlrpc_value *params; + int ret; + + TRACE("called"); + + xmlrpc_env_init(&env); + params = xmlrpc_build_value(&env, "()"); + if (va_rpc_has_error(&env)) { + return -1; + } + + ret = va_do_rpc(&env, "va.fsstatus", params, do_agent_fsstatus_cb, + cb, opaque); + if (ret) { + qerror_report(QERR_VA_FAILED, ret, strerror(ret)); + } + xmlrpc_DECREF(params); + return ret; +} diff --git a/virtagent.h b/virtagent.h index dba90d0..0d7575d 100644 --- a/virtagent.h +++ b/virtagent.h @@ -46,5 +46,14 @@ int do_agent_capabilities(Monitor *mon, const QDict *mon_params, MonitorCompletion cb, void *opaque); int va_client_init_capabilities(void); int va_send_hello(void); +void do_agent_fsfreeze_print(Monitor *mon, const QObject *qobject); +int do_agent_fsfreeze(Monitor *mon, const QDict *mon_params, + MonitorCompletion cb, void *opaque); +void do_agent_fsthaw_print(Monitor *mon, const QObject *qobject); +int do_agent_fsthaw(Monitor *mon, const QDict *mon_params, + MonitorCompletion cb, void *opaque); +void do_agent_fsstatus_print(Monitor *mon, const QObject *qobject); +int do_agent_fsstatus(Monitor *mon, const QDict *mon_params, + MonitorCompletion cb, void *opaque); #endif /* VIRTAGENT_H */ -- 1.7.3.5 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] Re: [PATCH v2 0/2] virtagent - fsfreeze support 2011-02-02 8:42 [Qemu-devel] [PATCH v2 0/2] virtagent - fsfreeze support Jes.Sorensen 2011-02-02 8:42 ` [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw Jes.Sorensen 2011-02-02 8:42 ` [Qemu-devel] [PATCH 2/2] Add monitor commands for fsfreeze support Jes.Sorensen @ 2011-02-03 18:15 ` Michael Roth 2 siblings, 0 replies; 22+ messages in thread From: Michael Roth @ 2011-02-03 18:15 UTC (permalink / raw) To: Jes.Sorensen; +Cc: lcapitulino, badari, qemu-devel, stefanha, agl Hi Jes, Can you add Badari (CC'd) to the next round? He's looked at potentially using virtagent for snapshots in the past and might have some insights here. Thanks. On 02/02/2011 02:42 AM, Jes.Sorensen@redhat.com wrote: > From: Jes Sorensen<Jes.Sorensen@redhat.com> > > Hi > > This is a first attempt to add fsfreeze support to virtagent. The idea > is for the guest agent to walk the list of locally mounted file > systems in the guest, and issuing an ioctl to freeze them. The host > can then do a live snapshot of the guest, obtaining stable file > systems. After the snapshot, the host then calls the thaw function in > virtagent, which goes through the list of previously frozen file > systems and unfreezes them. > > The list walking ignores remote file systems such as NFS and CIFS as > well as all pseudo file systems. > > The guest agent code is in the first patch, and host agent code is in > the second patch. For now there is only human monitor support, but it > should be pretty straight forward to add QMP support as well. > > Comments and suggestions welcome! > > v2 of the patch addresses the issues pointed out by Stefan and Michael. > > Cheers, > Jes > > > Jes Sorensen (2): > Add virtagent file system freeze/thaw > Add monitor commands for fsfreeze support > > hmp-commands.hx | 48 +++++++++++ > virtagent-common.h | 9 ++ > virtagent-server.c | 190 ++++++++++++++++++++++++++++++++++++++++++ > virtagent.c | 235 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > virtagent.h | 9 ++ > 5 files changed, 491 insertions(+), 0 deletions(-) > ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v3 0/2] virtagent - fsfreeze support @ 2011-02-04 10:57 Jes.Sorensen 2011-02-04 10:57 ` [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw Jes.Sorensen 0 siblings, 1 reply; 22+ messages in thread From: Jes.Sorensen @ 2011-02-04 10:57 UTC (permalink / raw) To: qemu-devel; +Cc: lcapitulino, badari, mdroth, stefanha, agl From: Jes Sorensen <Jes.Sorensen@redhat.com> Hi This is a first attempt to add fsfreeze support to virtagent. The idea is for the guest agent to walk the list of locally mounted file systems in the guest, and issuing an ioctl to freeze them. The host can then do a live snapshot of the guest, obtaining stable file systems. After the snapshot, the host then calls the thaw function in virtagent, which goes through the list of previously frozen file systems and unfreezes them. The list walking ignores remote file systems such as NFS and CIFS as well as all pseudo file systems. The guest agent code is in the first patch, and host agent code is in the second patch. For now there is only human monitor support, but it should be pretty straight forward to add QMP support as well. Comments and suggestions welcome! v3 of the patch encapsulates the freeze states in a struct and fixes some tab issues that I had missed. Both pointed out by Michael Roth. v2 of the patch addresses the issues pointed out by Stefan and Michael. Note I will be gone all of next week, so if I don't reply it's because I am busy skiing :) Cheers, Jes Jes Sorensen (2): Add virtagent file system freeze/thaw Add monitor commands for fsfreeze support hmp-commands.hx | 48 +++++++++++ virtagent-common.h | 9 ++ virtagent-server.c | 195 +++++++++++++++++++++++++++++++++++++++++++ virtagent.c | 235 ++++++++++++++++++++++++++++++++++++++++++++++++++++ virtagent.h | 9 ++ 5 files changed, 496 insertions(+), 0 deletions(-) -- 1.7.3.5 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw 2011-02-04 10:57 [Qemu-devel] [PATCH v3 " Jes.Sorensen @ 2011-02-04 10:57 ` Jes.Sorensen 0 siblings, 0 replies; 22+ messages in thread From: Jes.Sorensen @ 2011-02-04 10:57 UTC (permalink / raw) To: qemu-devel; +Cc: lcapitulino, badari, mdroth, stefanha, agl From: Jes Sorensen <Jes.Sorensen@redhat.com> 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. - fsfreeze(): Walk the list of mounted local real file systems, and freeze them. - fsthaw(): Walk the list of previously frozen file systems and thaw them. - fsstatus(): Return the current status of freeze/thaw. The host must poll this function, in case fsfreeze() returned with a timeout, to wait for the operation to finish. Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com> --- virtagent-common.h | 8 ++ virtagent-server.c | 195 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 203 insertions(+), 0 deletions(-) diff --git a/virtagent-common.h b/virtagent-common.h index 5d8f5c1..7c6d9ef 100644 --- a/virtagent-common.h +++ b/virtagent-common.h @@ -61,6 +61,14 @@ typedef struct VAContext { const char *channel_path; } VAContext; +enum va_fsfreeze_status { + FREEZE_ERROR = -1, + FREEZE_THAWED = 0, + FREEZE_INPROGRESS = 1, + FREEZE_FROZEN = 2, + FREEZE_THAWINPROGRESS = 3, +}; + enum va_job_status { VA_JOB_STATUS_PENDING = 0, VA_JOB_STATUS_OK, diff --git a/virtagent-server.c b/virtagent-server.c index 7bb35b2..5140918 100644 --- a/virtagent-server.c +++ b/virtagent-server.c @@ -14,6 +14,10 @@ #include <syslog.h> #include "qemu_socket.h" #include "virtagent-common.h" +#include <mntent.h> +#include <sys/types.h> +#include <sys/ioctl.h> +#include <linux/fs.h> static VAServerData *va_server_data; static bool va_enable_syslog = false; /* enable syslog'ing of RPCs */ @@ -217,6 +221,191 @@ static xmlrpc_value *va_hello(xmlrpc_env *env, return result; } + +/* + * Walk the mount table and build a list of local file systems + */ + +struct direntry { + char *dirname; + char *devtype; + struct direntry *next; +}; + +struct va_freezestate { + struct direntry *mount_list; + int status; +}; + +static struct va_freezestate freezestate; + +static int build_mount_list(void) +{ + struct mntent *mnt; + struct direntry *entry; + struct direntry *next; + char const *mtab = MOUNTED; + FILE *fp; + + fp = setmntent(mtab, "r"); + if (!fp) { + fprintf(stderr, "unable to read mtab\n"); + goto fail; + } + + while ((mnt = getmntent(fp))) { + /* + * An entry which device name doesn't start with a '/' is + * either a dummy file system or a network file system. + * Add special handling for smbfs and cifs as is done by + * coreutils as well. + */ + if ((mnt->mnt_fsname[0] != '/') || + (strcmp(mnt->mnt_type, "smbfs") == 0) || + (strcmp(mnt->mnt_type, "cifs") == 0)) { + continue; + } + + entry = qemu_malloc(sizeof(struct direntry)); + entry->dirname = qemu_strdup(mnt->mnt_dir); + entry->devtype = qemu_strdup(mnt->mnt_type); + entry->next = freezestate.mount_list; + + freezestate.mount_list = entry; + } + + endmntent(fp); + + return 0; + +fail: + while(freezestate.mount_list) { + next = freezestate.mount_list->next; + qemu_free(freezestate.mount_list->dirname); + qemu_free(freezestate.mount_list->devtype); + qemu_free(freezestate.mount_list); + freezestate.mount_list = next; + } + + return -1; +} + +/* + * va_fsfreeze(): Walk list of mounted file systems in the guest, and + * 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, + xmlrpc_value *params, + void *user_data) +{ + xmlrpc_int32 ret = 0, i = 0; + xmlrpc_value *result; + struct direntry *entry; + int fd; + SLOG("va_fsfreeze()"); + + if (freezestate.status != FREEZE_THAWED) { + ret = 0; + goto out; + } + + ret = build_mount_list(); + if (ret < 0) { + goto out; + } + + freezestate.status = FREEZE_INPROGRESS; + + entry = freezestate.mount_list; + while(entry) { + fd = qemu_open(entry->dirname, O_RDONLY); + if (fd == -1) { + ret = errno; + goto error; + } + ret = ioctl(fd, FIFREEZE); + close(fd); + if (ret < 0 && ret != EOPNOTSUPP) { + goto error; + } + + entry = entry->next; + i++; + } + + freezestate.status = FREEZE_FROZEN; + ret = i; +out: + result = xmlrpc_build_value(env, "i", ret); + return result; +error: + if (i > 0) { + freezestate.status = FREEZE_ERROR; + } + goto out; +} + +/* + * va_fsthaw(): Walk list of frozen file systems in the guest, and + * thaw them. + * rpc return values: Number of file systems thawed on success, -1 on error. + */ +static xmlrpc_value *va_fsthaw(xmlrpc_env *env, + xmlrpc_value *params, + void *user_data) +{ + xmlrpc_int32 ret; + xmlrpc_value *result; + struct direntry *entry; + int fd, i = 0; + SLOG("va_fsthaw()"); + + if (freezestate.status != FREEZE_FROZEN) { + ret = 0; + goto out; + } + + while((entry = freezestate.mount_list)) { + fd = qemu_open(entry->dirname, O_RDONLY); + if (fd == -1) { + ret = -1; + goto out; + } + ret = ioctl(fd, FITHAW); + close(fd); + if (ret < 0 && ret != EOPNOTSUPP) { + ret = -1; + goto out; + } + + freezestate.mount_list = entry->next; + qemu_free(entry->dirname); + qemu_free(entry->devtype); + qemu_free(entry); + i++; + } + + freezestate.status = FREEZE_THAWED; + ret = i; +out: + result = xmlrpc_build_value(env, "i", ret); + return result; +} + +/* + * va_fsstatus(): Return status of freeze/thaw + * rpc return values: fsfreeze_status + */ +static xmlrpc_value *va_fsstatus(xmlrpc_env *env, + xmlrpc_value *params, + void *user_data) +{ + xmlrpc_value *result = xmlrpc_build_value(env, "i", freezestate.status); + SLOG("va_fsstatus()"); + return result; +} + typedef struct RPCFunction { xmlrpc_value *(*func)(xmlrpc_env *env, xmlrpc_value *param, void *unused); const char *func_name; @@ -237,6 +426,12 @@ static RPCFunction guest_functions[] = { .func_name = "va.ping" }, { .func = va_capabilities, .func_name = "va.capabilities" }, + { .func = va_fsfreeze, + .func_name = "va.fsfreeze" }, + { .func = va_fsthaw, + .func_name = "va.fsthaw" }, + { .func = va_fsstatus, + .func_name = "va.fsstatus" }, { NULL, NULL } }; static RPCFunction host_functions[] = { -- 1.7.3.5 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 0/2] virtagent - fsfreeze support @ 2011-02-01 10:58 Jes.Sorensen 2011-02-01 10:58 ` [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw Jes.Sorensen 0 siblings, 1 reply; 22+ messages in thread From: Jes.Sorensen @ 2011-02-01 10:58 UTC (permalink / raw) To: qemu-devel; +Cc: mdroth, agl From: Jes Sorensen <Jes.Sorensen@redhat.com> Hi This is a first attempt to add fsfreeze support to virtagent. The idea is for the guest agent to walk the list of locally mounted file systems in the guest, and issuing an ioctl to freeze them. The host can then do a live snapshot of the guest, obtaining stable file systems. After the snapshot, the host then calls the thaw function in virtagent, which goes through the list of previously frozen file systems and unfreezes them. The list walking ignores remote file systems such as NFS and CIFS as well as all pseudo file systems. The guest agent code is in the first patch, and host agent code is in the second patch. For now there is only human monitor support, but it should be pretty straight forward to add QMP support as well. Patches are against the virtagent-dev git tree. Comments and suggestions welcome! Cheers, Jes Jes Sorensen (2): Add virtagent file system freeze/thaw Add monitor commands for fsfreeze support hmp-commands.hx | 48 +++++++++++ virtagent-common.h | 9 ++ virtagent-server.c | 196 +++++++++++++++++++++++++++++++++++++++++++ virtagent.c | 235 ++++++++++++++++++++++++++++++++++++++++++++++++++++ virtagent.h | 9 ++ 5 files changed, 497 insertions(+), 0 deletions(-) -- 1.7.3.5 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw 2011-02-01 10:58 [Qemu-devel] [PATCH 0/2] virtagent - fsfreeze support Jes.Sorensen @ 2011-02-01 10:58 ` Jes.Sorensen 2011-02-01 14:12 ` Stefan Hajnoczi 2011-02-02 7:57 ` Stefan Hajnoczi 0 siblings, 2 replies; 22+ messages in thread From: Jes.Sorensen @ 2011-02-01 10:58 UTC (permalink / raw) To: qemu-devel; +Cc: mdroth, agl From: Jes Sorensen <Jes.Sorensen@redhat.com> 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. - fsfreeze(): Walk the list of mounted local real file systems, and freeze them. - fsthaw(): Walk the list of previously frozen file systems and thaw them. - fsstatus(): Return the current status of freeze/thaw. The host must poll this function, in case fsfreeze() returned with a timeout, to wait for the operation to finish. Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com> --- virtagent-common.h | 8 ++ virtagent-server.c | 196 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 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 { const char *channel_path; } VAContext; +enum vs_fsfreeze_status { + FREEZE_ERROR = -1, + FREEZE_THAWED = 0, + FREEZE_INPROGRESS = 1, + FREEZE_FROZEN = 2, + FREEZE_THAWINPROGRESS = 3, +}; + enum va_job_status { VA_JOB_STATUS_PENDING = 0, 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 @@ #include <syslog.h> #include "qemu_socket.h" #include "virtagent-common.h" +#include <mntent.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <sys/errno.h> +#include <sys/ioctl.h> +#include <fcntl.h> +#include <linux/fs.h> static VAServerData *va_server_data; static bool va_enable_syslog = false; /* enable syslog'ing of RPCs */ @@ -217,6 +224,189 @@ static xmlrpc_value *va_hello(xmlrpc_env *env, return result; } + +/* + * Walk the mount table and build a list of local file systems + */ + +struct direntry { + char *dirname; + char *devtype; + struct direntry *next; +}; + +static struct direntry *mount_list; +static int fsfreeze_status; + +static int build_mount_list(void) +{ + struct mntent *mnt; + struct direntry *entry; + struct direntry *next; + char const *mtab = MOUNTED; + FILE *fp; + + fp = setmntent(mtab, "r"); + if (!fp) { + fprintf(stderr, "unable to read mtab\n"); + goto fail; + } + + while ((mnt = getmntent(fp))) { + /* + * An entry which device name doesn't start with a '/' is + * either a dummy file system or a network file system. + * Add special handling for smbfs and cifs as is done by + * coreutils as well. + */ + if ((mnt->mnt_fsname[0] != '/') || + (strcmp(mnt->mnt_type, "smbfs") == 0) || + (strcmp(mnt->mnt_type, "cifs") == 0)) { + continue; + } + + entry = qemu_malloc(sizeof(struct direntry)); + if (!entry) { + goto fail; + } + entry->dirname = qemu_strdup(mnt->mnt_dir); + entry->devtype = qemu_strdup(mnt->mnt_type); + entry->next = mount_list; + + mount_list = entry; + } + + endmntent(fp); + + return 0; + +fail: + while(mount_list) { + next = mount_list->next; + qemu_free(mount_list->dirname); + qemu_free(mount_list->devtype); + qemu_free(mount_list); + mount_list = next; + } + + return -1; +} + +/* + * va_fsfreeze(): Walk list of mounted file systems in the guest, and + * 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, + xmlrpc_value *params, + void *user_data) +{ + xmlrpc_int32 ret = 0, i = 0; + xmlrpc_value *result; + struct direntry *entry; + int fd; + SLOG("va_fsfreeze()"); + + if (fsfreeze_status == FREEZE_FROZEN) { + ret = 0; + goto out; + } + + ret = build_mount_list(); + if (ret < 0) { + goto out; + } + + fsfreeze_status = FREEZE_INPROGRESS; + + entry = mount_list; + while(entry) { + fd = qemu_open(entry->dirname, O_RDONLY); + if (fd == -1) { + ret = errno; + goto error; + } + ret = ioctl(fd, FIFREEZE); + if (ret < 0 && ret != EOPNOTSUPP) { + goto error; + } + + close(fd); + entry = entry->next; + i++; + } + + fsfreeze_status = FREEZE_FROZEN; + ret = i; +out: + result = xmlrpc_build_value(env, "i", ret); + return result; +error: + if (i > 0) { + fsfreeze_status = FREEZE_ERROR; + } + goto out; +} + +/* + * va_fsthaw(): Walk list of frozen file systems in the guest, and + * thaw them. + * rpc return values: Number of file systems thawed on success, -1 on error. + */ +static xmlrpc_value *va_fsthaw(xmlrpc_env *env, + xmlrpc_value *params, + void *user_data) +{ + xmlrpc_int32 ret; + xmlrpc_value *result; + struct direntry *entry; + int fd, i = 0; + SLOG("va_fsthaw()"); + + if (fsfreeze_status == FREEZE_THAWED) { + ret = 0; + goto out; + } + + while((entry = mount_list)) { + fd = qemu_open(entry->dirname, O_RDONLY); + if (fd == -1) { + ret = -1; + goto out; + } + ret = ioctl(fd, FITHAW); + if (ret < 0 && ret != EOPNOTSUPP) { + ret = -1; + goto out; + } + close(fd); + + mount_list = entry->next; + qemu_free(entry->dirname); + qemu_free(entry->devtype); + qemu_free(entry); + i++; + } + + fsfreeze_status = FREEZE_THAWED; + ret = i; +out: + result = xmlrpc_build_value(env, "i", ret); + return result; +} + +/* va_fsstatus(): Return status of freeze/thaw + * rpc return values: fsfreeze_status + */ +static xmlrpc_value *va_fsstatus(xmlrpc_env *env, + xmlrpc_value *params, + void *user_data) +{ + xmlrpc_value *result = xmlrpc_build_value(env, "i", fsfreeze_status); + SLOG("va_fsstatus()"); + return result; +} + typedef struct RPCFunction { xmlrpc_value *(*func)(xmlrpc_env *env, xmlrpc_value *param, void *unused); const char *func_name; @@ -237,6 +427,12 @@ static RPCFunction guest_functions[] = { .func_name = "va.ping" }, { .func = va_capabilities, .func_name = "va.capabilities" }, + { .func = va_fsfreeze, + .func_name = "va.fsfreeze" }, + { .func = va_fsthaw, + .func_name = "va.fsthaw" }, + { .func = va_fsstatus, + .func_name = "va.fsstatus" }, { NULL, NULL } }; static RPCFunction host_functions[] = { -- 1.7.3.5 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw 2011-02-01 10:58 ` [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw Jes.Sorensen @ 2011-02-01 14:12 ` Stefan Hajnoczi 2011-02-01 14:26 ` Jes Sorensen 2011-02-02 7:57 ` Stefan Hajnoczi 1 sibling, 1 reply; 22+ messages in thread From: Stefan Hajnoczi @ 2011-02-01 14:12 UTC (permalink / raw) To: Jes.Sorensen; +Cc: agl, qemu-devel, mdroth On Tue, Feb 1, 2011 at 10:58 AM, <Jes.Sorensen@redhat.com> wrote: > From: Jes Sorensen <Jes.Sorensen@redhat.com> > > 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. > - fsfreeze(): Walk the list of mounted local real file systems, > and freeze them. > - fsthaw(): Walk the list of previously frozen file systems and > thaw them. > - fsstatus(): Return the current status of freeze/thaw. The host must > poll this function, in case fsfreeze() returned with a > 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=vs.85).aspx > Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com> > --- > virtagent-common.h | 8 ++ > virtagent-server.c | 196 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 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 { > const char *channel_path; > } VAContext; > > +enum vs_fsfreeze_status { > + FREEZE_ERROR = -1, > + FREEZE_THAWED = 0, > + FREEZE_INPROGRESS = 1, > + FREEZE_FROZEN = 2, > + FREEZE_THAWINPROGRESS = 3, > +}; > + > enum va_job_status { > VA_JOB_STATUS_PENDING = 0, > 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 @@ > #include <syslog.h> > #include "qemu_socket.h" > #include "virtagent-common.h" > +#include <mntent.h> > +#include <sys/types.h> > +#include <sys/stat.h> > +#include <sys/errno.h> > +#include <sys/ioctl.h> > +#include <fcntl.h> > +#include <linux/fs.h> > > static VAServerData *va_server_data; > static bool va_enable_syslog = false; /* enable syslog'ing of RPCs */ > @@ -217,6 +224,189 @@ static xmlrpc_value *va_hello(xmlrpc_env *env, > return result; > } > > + > +/* > + * Walk the mount table and build a list of local file systems > + */ > + > +struct direntry { > + char *dirname; > + char *devtype; > + struct direntry *next; > +}; > + > +static struct direntry *mount_list; > +static int fsfreeze_status; > + > +static int build_mount_list(void) > +{ > + struct mntent *mnt; > + struct direntry *entry; > + struct direntry *next; > + char const *mtab = MOUNTED; > + FILE *fp; > + > + fp = setmntent(mtab, "r"); > + if (!fp) { > + fprintf(stderr, "unable to read mtab\n"); > + goto fail; > + } > + > + while ((mnt = getmntent(fp))) { > + /* > + * An entry which device name doesn't start with a '/' is > + * either a dummy file system or a network file system. > + * Add special handling for smbfs and cifs as is done by > + * coreutils as well. > + */ > + if ((mnt->mnt_fsname[0] != '/') || > + (strcmp(mnt->mnt_type, "smbfs") == 0) || > + (strcmp(mnt->mnt_type, "cifs") == 0)) { > + continue; > + } > + > + entry = qemu_malloc(sizeof(struct direntry)); > + if (!entry) { > + goto fail; > + } qemu_malloc() never fails. > + entry->dirname = qemu_strdup(mnt->mnt_dir); > + entry->devtype = qemu_strdup(mnt->mnt_type); > + entry->next = mount_list; > + > + mount_list = entry; > + } > + > + endmntent(fp); > + > + return 0; > + > +fail: > + while(mount_list) { > + next = mount_list->next; > + qemu_free(mount_list->dirname); > + qemu_free(mount_list->devtype); > + qemu_free(mount_list); > + mount_list = next; > + } > + > + return -1; > +} > + > +/* > + * va_fsfreeze(): Walk list of mounted file systems in the guest, and > + * 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, > + xmlrpc_value *params, > + void *user_data) > +{ > + xmlrpc_int32 ret = 0, i = 0; > + xmlrpc_value *result; > + struct direntry *entry; > + int fd; > + SLOG("va_fsfreeze()"); > + > + if (fsfreeze_status == FREEZE_FROZEN) { > + ret = 0; > + goto out; > + } The only valid status here is FREEZE_THAWED? Perhaps we should test for that specifically. > + > + ret = build_mount_list(); > + if (ret < 0) { > + goto out; > + } > + > + fsfreeze_status = FREEZE_INPROGRESS; > + > + entry = mount_list; > + while(entry) { > + fd = qemu_open(entry->dirname, O_RDONLY); > + if (fd == -1) { > + ret = errno; > + goto error; > + } > + ret = ioctl(fd, FIFREEZE); If you close(fd) here then it won't leak or need extra code in the error path. > + if (ret < 0 && ret != EOPNOTSUPP) { > + goto error; > + } > + > + close(fd); > + entry = entry->next; > + i++; > + } > + > + fsfreeze_status = FREEZE_FROZEN; > + ret = i; > +out: > + result = xmlrpc_build_value(env, "i", ret); > + return result; > +error: > + if (i > 0) { > + fsfreeze_status = FREEZE_ERROR; > + } > + goto out; > +} > + > +/* > + * va_fsthaw(): Walk list of frozen file systems in the guest, and > + * thaw them. > + * rpc return values: Number of file systems thawed on success, -1 on error. > + */ > +static xmlrpc_value *va_fsthaw(xmlrpc_env *env, > + xmlrpc_value *params, > + void *user_data) > +{ > + xmlrpc_int32 ret; > + xmlrpc_value *result; > + struct direntry *entry; > + int fd, i = 0; > + SLOG("va_fsthaw()"); > + > + if (fsfreeze_status == FREEZE_THAWED) { > + ret = 0; > + goto out; > + } A stricter check would be status FREEZE_FROZEN. > + > + while((entry = mount_list)) { > + fd = qemu_open(entry->dirname, O_RDONLY); > + if (fd == -1) { > + ret = -1; > + goto out; > + } > + ret = ioctl(fd, FITHAW); Same thing about close(fd) here. Stefan ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw 2011-02-01 14:12 ` Stefan Hajnoczi @ 2011-02-01 14:26 ` Jes Sorensen 2011-02-01 14:34 ` Stefan Hajnoczi 0 siblings, 1 reply; 22+ messages in thread From: Jes Sorensen @ 2011-02-01 14:26 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: agl, qemu-devel, mdroth On 02/01/11 15:12, Stefan Hajnoczi wrote: > On Tue, Feb 1, 2011 at 10:58 AM, <Jes.Sorensen@redhat.com> wrote: >> From: Jes Sorensen <Jes.Sorensen@redhat.com> >> >> 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. >> - fsfreeze(): Walk the list of mounted local real file systems, >> and freeze them. >> - fsthaw(): Walk the list of previously frozen file systems and >> thaw them. >> - fsstatus(): Return the current status of freeze/thaw. The host must >> poll this function, in case fsfreeze() returned with a >> 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? I have to admit you lost me here, where do you get that 500ms time from? Is that the XMLRPC polling time or? I just used the example code from other agent calls. > 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=vs.85).aspx I haven't looked at it, I designed the calls based on how they fit with the Linux ioctls. >> + entry = qemu_malloc(sizeof(struct direntry)); >> + if (!entry) { >> + goto fail; >> + } > > qemu_malloc() never fails. Good point, we have ugly malloc in qemu :( I wrote the code to handle this outside QEMU first, to make sure it worked correctly and trying to see how many times I could crash my laptop in the process. I'll fix it. >> +static xmlrpc_value *va_fsfreeze(xmlrpc_env *env, >> + xmlrpc_value *params, >> + void *user_data) >> +{ >> + xmlrpc_int32 ret = 0, i = 0; >> + xmlrpc_value *result; >> + struct direntry *entry; >> + int fd; >> + SLOG("va_fsfreeze()"); >> + >> + if (fsfreeze_status == FREEZE_FROZEN) { >> + ret = 0; >> + goto out; >> + } > > The only valid status here is FREEZE_THAWED? Perhaps we should test > for that specifically. Good point, I'll fix this. >> + >> + ret = build_mount_list(); >> + if (ret < 0) { >> + goto out; >> + } >> + >> + fsfreeze_status = FREEZE_INPROGRESS; >> + >> + entry = mount_list; >> + while(entry) { >> + fd = qemu_open(entry->dirname, O_RDONLY); >> + if (fd == -1) { >> + ret = errno; >> + goto error; >> + } >> + ret = ioctl(fd, FIFREEZE); > > If you close(fd) here then it won't leak or need extra code in the error path. Good point, will fix. >> +static xmlrpc_value *va_fsthaw(xmlrpc_env *env, >> + xmlrpc_value *params, >> + void *user_data) >> +{ >> + xmlrpc_int32 ret; >> + xmlrpc_value *result; >> + struct direntry *entry; >> + int fd, i = 0; >> + SLOG("va_fsthaw()"); >> + >> + if (fsfreeze_status == FREEZE_THAWED) { >> + ret = 0; >> + goto out; >> + } > > A stricter check would be status FREEZE_FROZEN. Yep, will fix >> + >> + while((entry = mount_list)) { >> + fd = qemu_open(entry->dirname, O_RDONLY); >> + if (fd == -1) { >> + ret = -1; >> + goto out; >> + } >> + ret = ioctl(fd, FITHAW); > > Same thing about close(fd) here. Thanks for the review, all valid points! Cheers, Jes ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw 2011-02-01 14:26 ` Jes Sorensen @ 2011-02-01 14:34 ` Stefan Hajnoczi 2011-02-01 14:36 ` Jes Sorensen 0 siblings, 1 reply; 22+ messages in thread From: Stefan Hajnoczi @ 2011-02-01 14:34 UTC (permalink / raw) To: Jes Sorensen; +Cc: agl, qemu-devel, mdroth On Tue, Feb 1, 2011 at 2:26 PM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote: > On 02/01/11 15:12, Stefan Hajnoczi wrote: >> On Tue, Feb 1, 2011 at 10:58 AM, <Jes.Sorensen@redhat.com> wrote: >>> From: Jes Sorensen <Jes.Sorensen@redhat.com> >>> >>> 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. >>> - fsfreeze(): Walk the list of mounted local real file systems, >>> and freeze them. >>> - fsthaw(): Walk the list of previously frozen file systems and >>> thaw them. >>> - fsstatus(): Return the current status of freeze/thaw. The host must >>> poll this function, in case fsfreeze() returned with a >>> 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? > > I have to admit you lost me here, where do you get that 500ms time from? > Is that the XMLRPC polling time or? I just used the example code from > other agent calls. 500 ms is made up. I was thinking, "what would a reasonable polling interval be?" and picked a sub-second number. Can you explain how the timeout in fsfreeze can happen? It's probably because I don't know the virtagent details. Stefan ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw 2011-02-01 14:34 ` Stefan Hajnoczi @ 2011-02-01 14:36 ` Jes Sorensen 2011-02-01 14:41 ` Stefan Hajnoczi 0 siblings, 1 reply; 22+ messages in thread From: Jes Sorensen @ 2011-02-01 14:36 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: agl, qemu-devel, mdroth On 02/01/11 15:34, Stefan Hajnoczi wrote: > On Tue, Feb 1, 2011 at 2:26 PM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote: >> I have to admit you lost me here, where do you get that 500ms time from? >> Is that the XMLRPC polling time or? I just used the example code from >> other agent calls. > > 500 ms is made up. I was thinking, "what would a reasonable polling > interval be?" and picked a sub-second number. > > Can you explain how the timeout in fsfreeze can happen? It's probably > because I don't know the virtagent details. Ah ok. >From what I understand, the XMLRPC code is setup to timeout if the guest doesn't reply within a certain amount of time. In that case, the caller needs to poll to wait for the guest to complete the freeze. This really should only happen if you have a guest with a large number of very large file systems. I don't know how likely it is to happen in real life. Cheers, Jes ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw 2011-02-01 14:36 ` Jes Sorensen @ 2011-02-01 14:41 ` Stefan Hajnoczi 2011-02-01 17:22 ` Michael Roth 0 siblings, 1 reply; 22+ messages in thread From: Stefan Hajnoczi @ 2011-02-01 14:41 UTC (permalink / raw) To: mdroth; +Cc: Jes Sorensen, qemu-devel, agl On Tue, Feb 1, 2011 at 2:36 PM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote: > On 02/01/11 15:34, Stefan Hajnoczi wrote: >> On Tue, Feb 1, 2011 at 2:26 PM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote: >>> I have to admit you lost me here, where do you get that 500ms time from? >>> Is that the XMLRPC polling time or? I just used the example code from >>> other agent calls. >> >> 500 ms is made up. I was thinking, "what would a reasonable polling >> interval be?" and picked a sub-second number. >> >> Can you explain how the timeout in fsfreeze can happen? It's probably >> because I don't know the virtagent details. > > Ah ok. > > From what I understand, the XMLRPC code is setup to timeout if the guest > doesn't reply within a certain amount of time. In that case, the caller > needs to poll to wait for the guest to complete the freeze. This really > should only happen if you have a guest with a large number of very large > file systems. I don't know how likely it is to happen in real life. Perhaps Michael can confirm that the freeze function continues to execute after timeout but the client is able to send fsstatus() requests? Stefan ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw 2011-02-01 14:41 ` Stefan Hajnoczi @ 2011-02-01 17:22 ` Michael Roth 0 siblings, 0 replies; 22+ messages in thread From: Michael Roth @ 2011-02-01 17:22 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: Jes Sorensen, qemu-devel, agl On 02/01/2011 08:41 AM, Stefan Hajnoczi wrote: > On Tue, Feb 1, 2011 at 2:36 PM, Jes Sorensen<Jes.Sorensen@redhat.com> wrote: >> On 02/01/11 15:34, Stefan Hajnoczi wrote: >>> On Tue, Feb 1, 2011 at 2:26 PM, Jes Sorensen<Jes.Sorensen@redhat.com> wrote: >>>> I have to admit you lost me here, where do you get that 500ms time from? >>>> Is that the XMLRPC polling time or? I just used the example code from >>>> other agent calls. >>> >>> 500 ms is made up. I was thinking, "what would a reasonable polling >>> interval be?" and picked a sub-second number. >>> >>> Can you explain how the timeout in fsfreeze can happen? It's probably >>> because I don't know the virtagent details. >> >> Ah ok. >> >> From what I understand, the XMLRPC code is setup to timeout if the guest >> doesn't reply within a certain amount of time. In that case, the caller >> needs to poll to wait for the guest to complete the freeze. This really >> should only happen if you have a guest with a large number of very large >> file systems. I don't know how likely it is to happen in real life. > > Perhaps Michael can confirm that the freeze function continues to > execute after timeout but the client is able to send fsstatus() > requests? Ahh, yeah there's the confusion: we only execute one RPC at a time, so a polling function for a previous RPC won't work unless that RPC is being done concurrently, via fork()ing or something and communicating status via some method of IPC. I touched on possible approaches to dealing with this in the response I just sent to this patch. > > Stefan ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw 2011-02-01 10:58 ` [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw Jes.Sorensen 2011-02-01 14:12 ` Stefan Hajnoczi @ 2011-02-02 7:57 ` Stefan Hajnoczi 2011-02-02 8:48 ` Jes Sorensen 1 sibling, 1 reply; 22+ messages in thread From: Stefan Hajnoczi @ 2011-02-02 7:57 UTC (permalink / raw) To: Jes.Sorensen; +Cc: agl, qemu-devel, mdroth On Tue, Feb 1, 2011 at 10:58 AM, <Jes.Sorensen@redhat.com> wrote: > From: Jes Sorensen <Jes.Sorensen@redhat.com> > > 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. > - fsfreeze(): Walk the list of mounted local real file systems, > and freeze them. Does this add a requirement that guest agent code issues no disk I/O in its main loop (e.g. logging)? Otherwise we might deadlock ourselves waiting for I/O which is never issued. Stefan ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw 2011-02-02 7:57 ` Stefan Hajnoczi @ 2011-02-02 8:48 ` Jes Sorensen 2011-02-03 17:41 ` Michael Roth 0 siblings, 1 reply; 22+ messages in thread From: Jes Sorensen @ 2011-02-02 8:48 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: agl, qemu-devel, mdroth On 02/02/11 08:57, Stefan Hajnoczi wrote: > On Tue, Feb 1, 2011 at 10:58 AM, <Jes.Sorensen@redhat.com> wrote: >> From: Jes Sorensen <Jes.Sorensen@redhat.com> >> >> 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. >> - fsfreeze(): Walk the list of mounted local real file systems, >> and freeze them. > > Does this add a requirement that guest agent code issues no disk I/O > in its main loop (e.g. logging)? Otherwise we might deadlock > ourselves waiting for I/O which is never issued. Yes very much so[1] - one reason why it would be nice to have virtagent use threads to execute the actual commands. We should probably add a flag to agent commands indicating whether they issue disk I/O or not, so we can block attempts to execute commands that do so, while the guest is frozen. Cheers, Jes [1] speaking from experience ... a Linux desktop gets really upset if you freeze the file systems from a command in an xterm.... ho hum ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw 2011-02-02 8:48 ` Jes Sorensen @ 2011-02-03 17:41 ` Michael Roth 2011-02-04 6:13 ` Stefan Hajnoczi 2011-02-04 11:03 ` Jes Sorensen 0 siblings, 2 replies; 22+ messages in thread From: Michael Roth @ 2011-02-03 17:41 UTC (permalink / raw) To: Jes Sorensen; +Cc: Stefan Hajnoczi, qemu-devel, agl On 02/02/2011 02:48 AM, Jes Sorensen wrote: > On 02/02/11 08:57, Stefan Hajnoczi wrote: >> On Tue, Feb 1, 2011 at 10:58 AM,<Jes.Sorensen@redhat.com> wrote: >>> From: Jes Sorensen<Jes.Sorensen@redhat.com> >>> >>> 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. >>> - fsfreeze(): Walk the list of mounted local real file systems, >>> and freeze them. >> >> Does this add a requirement that guest agent code issues no disk I/O >> in its main loop (e.g. logging)? Otherwise we might deadlock >> ourselves waiting for I/O which is never issued. > > Yes very much so[1] - one reason why it would be nice to have virtagent > use threads to execute the actual commands. We should probably add a > flag to agent commands indicating whether they issue disk I/O or not, so > we can block attempts to execute commands that do so, while the guest is > frozen. **Warning, epic response** For things like logging and i/o on a frozen system...I agree we'd need some flag for these kinds of situations. Maybe a disable_logging() flag....i really don't like this though... I'd imagine even syslogd() could block virtagent in this type of situation, so that would need to be disabled as well. But doing so completely subverts our attempts and providing proper accounting of what the agent is doing to the user. A user can freeze the filesystem, knowing that logging would be disabled, then prod at whatever he wants. So the handling should be something specific to fsfreeze, with stricter requirements: If a user calls fsfreeze(), we disable logging, but also disable the ability to do anything other than fsthaw() or fsstatus(). This actually solves the potential deadlocking problem for other RPCs as well...since they cant be executed in the first place. So I think that addresses the agent deadlocking itself, post-freeze. However, fsfreeze() itself might lock-up the agent as well...I'm not confident we can really put any kind of bound on how long it'll take to execute, and if we timeout on the client-side the agent can still block here. Plus there are any number of other situations where an RPC can still hang things...in the future when we potentially allow things like script execution, they might do something like attempt to connect to a socket that's already in use and wait on the server for an arbitrary amount of time, or open a file on an nfs share that in currently unresponsive. So a solution for these situations is still needed, and I'm starting to agree that threads are needed, but I don't think we should do RPCs concurrently (not sure if that's what is being suggested or not). At least, there's no pressing reason for it as things currently stand (there aren't currently any RPCs where fast response times are all that important, so it's okay to serialize them behind previous RPCs, and HMP/QMP are command at a time), and it's something that Im fairly confident can be added if the need arises in the future. But for dealing with a situation where an RPC can hang the agent, I think one thread should do it. Basically: We associate each RPC with a time limit. Some RPCs, very special ones that we'd trust with our kids, could potentially specify an unlimited timeout. The client side should use this same timeout on it's end. In the future we might allow the user to explicitly disable the timeout for a certain RPC. The logic would then be: - read in a client RPC request - start a thread to do RPC - if there's a timeout, register an alarm(<timeout>), with a handler that will call something like pthread_kill(current_worker_thread). On the thread side, this signal will induce a pthread_exit() - wait for the thread to return (pthread_join(current_worker_thread)) - return it's response back to the caller if it finished, return a timeout indication otherwise > > Cheers, > Jes > > [1] speaking from experience ... a Linux desktop gets really upset if > you freeze the file systems from a command in an xterm.... ho hum ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw 2011-02-03 17:41 ` Michael Roth @ 2011-02-04 6:13 ` Stefan Hajnoczi 2011-02-04 16:27 ` Michael Roth 2011-02-04 11:03 ` Jes Sorensen 1 sibling, 1 reply; 22+ messages in thread From: Stefan Hajnoczi @ 2011-02-04 6:13 UTC (permalink / raw) To: Michael Roth; +Cc: Jes Sorensen, qemu-devel, agl On Thu, Feb 3, 2011 at 5:41 PM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote: > For things like logging and i/o on a frozen system...I agree we'd need some > flag for these kinds of situations. Maybe a disable_logging() flag....i > really don't like this though... I'd imagine even syslogd() could block > virtagent in this type of situation, so that would need to be disabled as > well. > > But doing so completely subverts our attempts and providing proper > accounting of what the agent is doing to the user. A user can freeze the > filesystem, knowing that logging would be disabled, then prod at whatever he > wants. So the handling should be something specific to fsfreeze, with > stricter requirements: > > If a user calls fsfreeze(), we disable logging, but also disable the ability > to do anything other than fsthaw() or fsstatus(). This actually solves the > potential deadlocking problem for other RPCs as well...since they cant be > executed in the first place. > > So I think that addresses the agent deadlocking itself, post-freeze. > > However, fsfreeze() itself might lock-up the agent as well...I'm not > confident we can really put any kind of bound on how long it'll take to > execute, and if we timeout on the client-side the agent can still block > here. > > Plus there are any number of other situations where an RPC can still hang > things...in the future when we potentially allow things like script > execution, they might do something like attempt to connect to a socket > that's already in use and wait on the server for an arbitrary amount of > time, or open a file on an nfs share that in currently unresponsive. > > So a solution for these situations is still needed, and I'm starting to > agree that threads are needed, but I don't think we should do RPCs > concurrently (not sure if that's what is being suggested or not). At least, > there's no pressing reason for it as things currently stand (there aren't > currently any RPCs where fast response times are all that important, so it's > okay to serialize them behind previous RPCs, and HMP/QMP are command at a > time), and it's something that Im fairly confident can be added if the need > arises in the future. > > But for dealing with a situation where an RPC can hang the agent, I think > one thread should do it. Basically: > > We associate each RPC with a time limit. Some RPCs, very special ones that > we'd trust with our kids, could potentially specify an unlimited timeout. > The client side should use this same timeout on it's end. In the future we > might allow the user to explicitly disable the timeout for a certain RPC. > The logic would then be: > > - read in a client RPC request > - start a thread to do RPC > - if there's a timeout, register an alarm(<timeout>), with a handler that > will call something like pthread_kill(current_worker_thread). On the thread > side, this signal will induce a pthread_exit() > - wait for the thread to return (pthread_join(current_worker_thread)) > - return it's response back to the caller if it finished, return a timeout > indication otherwise I'm not sure about a timeout inside virtagent. A client needs to protect itself with its own timeout and shouldn't rely on the server to prevent it from locking up - especially since the server is a guest which we have no control over. So the timeout does not help the guest. Aborting an RPC handler could leave the system in an inconsistent state unless we are careful. For example, aborting freeze requires thawing those file systems that have been successfully frozen so far. For other handlers it might leave temporary files around, or if they are not carefully written may partially update files in-place and leave them corrupted. So instead of a blanket timeout, I think handlers that perform operations that may block for unknown periods of time could specifically use timeouts. That gives the handler control to perform cleanup. Stefan ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw 2011-02-04 6:13 ` Stefan Hajnoczi @ 2011-02-04 16:27 ` Michael Roth 2011-02-04 16:52 ` Stefan Hajnoczi 0 siblings, 1 reply; 22+ messages in thread From: Michael Roth @ 2011-02-04 16:27 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: Jes Sorensen, qemu-devel, agl On 02/04/2011 12:13 AM, Stefan Hajnoczi wrote: > On Thu, Feb 3, 2011 at 5:41 PM, Michael Roth<mdroth@linux.vnet.ibm.com> wrote: >> For things like logging and i/o on a frozen system...I agree we'd need some >> flag for these kinds of situations. Maybe a disable_logging() flag....i >> really don't like this though... I'd imagine even syslogd() could block >> virtagent in this type of situation, so that would need to be disabled as >> well. >> >> But doing so completely subverts our attempts and providing proper >> accounting of what the agent is doing to the user. A user can freeze the >> filesystem, knowing that logging would be disabled, then prod at whatever he >> wants. So the handling should be something specific to fsfreeze, with >> stricter requirements: >> >> If a user calls fsfreeze(), we disable logging, but also disable the ability >> to do anything other than fsthaw() or fsstatus(). This actually solves the >> potential deadlocking problem for other RPCs as well...since they cant be >> executed in the first place. >> >> So I think that addresses the agent deadlocking itself, post-freeze. >> >> However, fsfreeze() itself might lock-up the agent as well...I'm not >> confident we can really put any kind of bound on how long it'll take to >> execute, and if we timeout on the client-side the agent can still block >> here. >> >> Plus there are any number of other situations where an RPC can still hang >> things...in the future when we potentially allow things like script >> execution, they might do something like attempt to connect to a socket >> that's already in use and wait on the server for an arbitrary amount of >> time, or open a file on an nfs share that in currently unresponsive. >> >> So a solution for these situations is still needed, and I'm starting to >> agree that threads are needed, but I don't think we should do RPCs >> concurrently (not sure if that's what is being suggested or not). At least, >> there's no pressing reason for it as things currently stand (there aren't >> currently any RPCs where fast response times are all that important, so it's >> okay to serialize them behind previous RPCs, and HMP/QMP are command at a >> time), and it's something that Im fairly confident can be added if the need >> arises in the future. >> >> But for dealing with a situation where an RPC can hang the agent, I think >> one thread should do it. Basically: >> >> We associate each RPC with a time limit. Some RPCs, very special ones that >> we'd trust with our kids, could potentially specify an unlimited timeout. >> The client side should use this same timeout on it's end. In the future we >> might allow the user to explicitly disable the timeout for a certain RPC. >> The logic would then be: >> >> - read in a client RPC request >> - start a thread to do RPC >> - if there's a timeout, register an alarm(<timeout>), with a handler that >> will call something like pthread_kill(current_worker_thread). On the thread >> side, this signal will induce a pthread_exit() >> - wait for the thread to return (pthread_join(current_worker_thread)) >> - return it's response back to the caller if it finished, return a timeout >> indication otherwise > > I'm not sure about a timeout inside virtagent. A client needs to > protect itself with its own timeout and shouldn't rely on the server > to prevent it from locking up - especially since the server is a guest > which we have no control over. So the timeout does not help the > guest. We actually have timeouts for the client already (though they'll need to be reworked a bit to handle the proposed solutions), what I'm proposing is an additional timeout on the guest/server side for the actual RPCs, since a blocking RPC can still hang the guest agent. > > Aborting an RPC handler could leave the system in an inconsistent > state unless we are careful. For example, aborting freeze requires > thawing those file systems that have been successfully frozen so far. > For other handlers it might leave temporary files around, or if they > are not carefully written may partially update files in-place and > leave them corrupted. > > So instead of a blanket timeout, I think handlers that perform > operations that may block for unknown periods of time could > specifically use timeouts. That gives the handler control to perform > cleanup. Good point. Although, I'm not sure I want to push timeout handling to the actual RPCs though....something as simple as open()/read() can block indefinitely in certain situations, and it'll be difficult to account for every situation, and the resulting code will be tedious as well. I'd really like the make the actual RPC as simple as possible, since it's something that may be extended heavily over time. So what if we simply allow an RPC to register a timeout handler at the beginning of the RPC call? So when the thread doing the RPC exits we: - check to see if thread exited as a result of timeout - check to see if a timeout handler was registered, if so, call it, reset the handler, then return a timeout indication - if it didn't time out, return the response The only burden this puts on the RPC author is that information they need to recover state would need to be accessible outside the thread, which is easily done by encapsulating state in static/global structs. So the timeout handler for fsfreeze, as it is currently written, would be something like: va_fsfreeze_timeout_handler(): foreach mnt in fsfreeze.mount_list: unfreeze(mnt) fsfreeze.mount_list = NULL We'll need to be careful about lists/objects being in weird states due to the forced exit, but I think it's doable. > > Stefan ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw 2011-02-04 16:27 ` Michael Roth @ 2011-02-04 16:52 ` Stefan Hajnoczi 0 siblings, 0 replies; 22+ messages in thread From: Stefan Hajnoczi @ 2011-02-04 16:52 UTC (permalink / raw) To: Michael Roth; +Cc: Jes Sorensen, qemu-devel, agl On Fri, Feb 4, 2011 at 4:27 PM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote: >> Aborting an RPC handler could leave the system in an inconsistent >> state unless we are careful. For example, aborting freeze requires >> thawing those file systems that have been successfully frozen so far. >> For other handlers it might leave temporary files around, or if they >> are not carefully written may partially update files in-place and >> leave them corrupted. >> >> So instead of a blanket timeout, I think handlers that perform >> operations that may block for unknown periods of time could >> specifically use timeouts. That gives the handler control to perform >> cleanup. > > Good point. Although, I'm not sure I want to push timeout handling to the > actual RPCs though....something as simple as open()/read() can block > indefinitely in certain situations, and it'll be difficult to account for > every situation, and the resulting code will be tedious as well. I'd really > like the make the actual RPC as simple as possible, since it's something > that may be extended heavily over time. > > So what if we simply allow an RPC to register a timeout handler at the > beginning of the RPC call? So when the thread doing the RPC exits we: > > - check to see if thread exited as a result of timeout > - check to see if a timeout handler was registered, if so, call it, reset > the handler, then return a timeout indication > - if it didn't time out, return the response > > The only burden this puts on the RPC author is that information they need to > recover state would need to be accessible outside the thread, which is > easily done by encapsulating state in static/global structs. So the timeout > handler for fsfreeze, as it is currently written, would be something like: > > va_fsfreeze_timeout_handler(): > foreach mnt in fsfreeze.mount_list: > unfreeze(mnt) > fsfreeze.mount_list = NULL > > We'll need to be careful about lists/objects being in weird states due to > the forced exit, but I think it's doable. Yeah, still requires discipline but it could work. Stefan ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw 2011-02-03 17:41 ` Michael Roth 2011-02-04 6:13 ` Stefan Hajnoczi @ 2011-02-04 11:03 ` Jes Sorensen 2011-02-04 16:51 ` Michael Roth 1 sibling, 1 reply; 22+ messages in thread From: Jes Sorensen @ 2011-02-04 11:03 UTC (permalink / raw) To: Michael Roth; +Cc: Stefan Hajnoczi, qemu-devel, agl On 02/03/11 18:41, Michael Roth wrote: > On 02/02/2011 02:48 AM, Jes Sorensen wrote: >> Yes very much so[1] - one reason why it would be nice to have virtagent >> use threads to execute the actual commands. We should probably add a >> flag to agent commands indicating whether they issue disk I/O or not, so >> we can block attempts to execute commands that do so, while the guest is >> frozen. > > **Warning, epic response** > > For things like logging and i/o on a frozen system...I agree we'd need > some flag for these kinds of situations. Maybe a disable_logging() > flag....i really don't like this though... I'd imagine even syslogd() > could block virtagent in this type of situation, so that would need to > be disabled as well. One way to resolve this would be to have the logging handled in it's own thread, which uses non blocking calls to do the actual logging. Obviously we'd have to use a non file system based communication method between the main thread and the logging thread :) > But doing so completely subverts our attempts and providing proper > accounting of what the agent is doing to the user. A user can freeze the > filesystem, knowing that logging would be disabled, then prod at > whatever he wants. So the handling should be something specific to > fsfreeze, with stricter requirements: > > If a user calls fsfreeze(), we disable logging, but also disable the > ability to do anything other than fsthaw() or fsstatus(). This actually > solves the potential deadlocking problem for other RPCs as well...since > they cant be executed in the first place. I disagree that we should block all calls, except for fsfreeze/fsstatus/ fsthaw in this case. There are other calls that could be valid in this situations, so I think it needs to be evaluated on a case by case basis. > So a solution for these situations is still needed, and I'm starting to > agree that threads are needed, but I don't think we should do RPCs > concurrently (not sure if that's what is being suggested or not). At > least, there's no pressing reason for it as things currently stand > (there aren't currently any RPCs where fast response times are all that > important, so it's okay to serialize them behind previous RPCs, and > HMP/QMP are command at a time), and it's something that Im fairly > confident can be added if the need arises in the future. Eventually I think we will need to be able to support concurrent RPC calls. There can be situations where an operation takes a long time while it is valid to be able to ping the guest agent to verify that it is still alive etc. Cheers, Jes ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw 2011-02-04 11:03 ` Jes Sorensen @ 2011-02-04 16:51 ` Michael Roth 0 siblings, 0 replies; 22+ messages in thread From: Michael Roth @ 2011-02-04 16:51 UTC (permalink / raw) To: Jes Sorensen; +Cc: Stefan Hajnoczi, qemu-devel, agl On 02/04/2011 05:03 AM, Jes Sorensen wrote: > On 02/03/11 18:41, Michael Roth wrote: >> On 02/02/2011 02:48 AM, Jes Sorensen wrote: >>> Yes very much so[1] - one reason why it would be nice to have virtagent >>> use threads to execute the actual commands. We should probably add a >>> flag to agent commands indicating whether they issue disk I/O or not, so >>> we can block attempts to execute commands that do so, while the guest is >>> frozen. >> >> **Warning, epic response** >> >> For things like logging and i/o on a frozen system...I agree we'd need >> some flag for these kinds of situations. Maybe a disable_logging() >> flag....i really don't like this though... I'd imagine even syslogd() >> could block virtagent in this type of situation, so that would need to >> be disabled as well. > > One way to resolve this would be to have the logging handled in it's own > thread, which uses non blocking calls to do the actual logging. > Obviously we'd have to use a non file system based communication method > between the main thread and the logging thread :) > I suspect syslogd() already buffers to some extent. But the problem there, as well the problem with having our own buffered logging implementation, is that we can't rely on being able to log an arbitrary number of messages. As some point that interface would need to either block, or stop dropping log messages. If it blocks, we're deadlocked again. If it drops messages, it's trivial for someone to flood it with messages after the fsfreeze() to get back into a state where they can execute RPC without any accounting. So a seperate logging thread doesn't buy us much, and what we come up with is rely gonna come down to syslogd: 1) if syslogd blocks, we must disable logging after fsfreeze(). Buffering, on syslogd's side or our own, only buys us time. If we disable logging, we must only allow absolutely required/benign RPCs. fsstatus/fsthaw are required, things like va.ping are benign, and potentially useful, in this particular situation. copyfile/shutdown/exec/etc should be considered "high-risk"...we want to make sure these are logged. 2) if syslogd doesnt block, it either drops messages at some point with no indication to us, or it drops them and provides some indication. If there's no indication, we must treat this the same way we treat 1), since we must assume that logging is effectively disabled. So only required or benign RPCs. if we get some indication when a call to syslog() fails to log/buffer, we can allow all RPCs, but treat failures to log as a cue to immediately cleanup and exit the RPC. fsfreeze() under this circumstance will need to make sure it only logs after it does the unfreeze, else we may never be able to unfreeze at that point forward. So the solution is dependent on syslogd's behavior. Ill have to do some testing to confirm...but I think the above covers the possibilities. >> But doing so completely subverts our attempts and providing proper >> accounting of what the agent is doing to the user. A user can freeze the >> filesystem, knowing that logging would be disabled, then prod at >> whatever he wants. So the handling should be something specific to >> fsfreeze, with stricter requirements: >> >> If a user calls fsfreeze(), we disable logging, but also disable the >> ability to do anything other than fsthaw() or fsstatus(). This actually >> solves the potential deadlocking problem for other RPCs as well...since >> they cant be executed in the first place. > > I disagree that we should block all calls, except for fsfreeze/fsstatus/ > fsthaw in this case. There are other calls that could be valid in this > situations, so I think it needs to be evaluated on a case by case basis. > >> So a solution for these situations is still needed, and I'm starting to >> agree that threads are needed, but I don't think we should do RPCs >> concurrently (not sure if that's what is being suggested or not). At >> least, there's no pressing reason for it as things currently stand >> (there aren't currently any RPCs where fast response times are all that >> important, so it's okay to serialize them behind previous RPCs, and >> HMP/QMP are command at a time), and it's something that Im fairly >> confident can be added if the need arises in the future. > > Eventually I think we will need to be able to support concurrent RPC > calls. There can be situations where an operation takes a long time > while it is valid to be able to ping the guest agent to verify that it > is still alive etc. Currently we're limited to 1 RPC at a time by the monitor/hmp/qmp (except for some niche cases where virtagent has multiple requests in flight). Ideally this will remain the interface we expose to users, but there could be situations where this isn't the case...for instance, other bits of qemu making direct calls into virtagent. I think we can add support for concurrent RPCs incrementally though...I've thought over the implementation details a bit and it seems to be fairly straightforward. I think we need to get the basic use cases down first though. > > Cheers, > Jes > ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2011-02-04 16:52 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-02-02 8:42 [Qemu-devel] [PATCH v2 0/2] virtagent - fsfreeze support Jes.Sorensen 2011-02-02 8:42 ` [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw Jes.Sorensen 2011-02-03 18:11 ` [Qemu-devel] " Michael Roth 2011-02-04 10:54 ` Jes Sorensen 2011-02-02 8:42 ` [Qemu-devel] [PATCH 2/2] Add monitor commands for fsfreeze support Jes.Sorensen 2011-02-03 18:15 ` [Qemu-devel] Re: [PATCH v2 0/2] virtagent - " Michael Roth -- strict thread matches above, loose matches on Subject: below -- 2011-02-04 10:57 [Qemu-devel] [PATCH v3 " Jes.Sorensen 2011-02-04 10:57 ` [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw Jes.Sorensen 2011-02-01 10:58 [Qemu-devel] [PATCH 0/2] virtagent - fsfreeze support Jes.Sorensen 2011-02-01 10:58 ` [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw Jes.Sorensen 2011-02-01 14:12 ` Stefan Hajnoczi 2011-02-01 14:26 ` Jes Sorensen 2011-02-01 14:34 ` Stefan Hajnoczi 2011-02-01 14:36 ` Jes Sorensen 2011-02-01 14:41 ` Stefan Hajnoczi 2011-02-01 17:22 ` Michael Roth 2011-02-02 7:57 ` Stefan Hajnoczi 2011-02-02 8:48 ` Jes Sorensen 2011-02-03 17:41 ` Michael Roth 2011-02-04 6:13 ` Stefan Hajnoczi 2011-02-04 16:27 ` Michael Roth 2011-02-04 16:52 ` Stefan Hajnoczi 2011-02-04 11:03 ` Jes Sorensen 2011-02-04 16:51 ` Michael Roth
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.