[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH 1/2] Add virtagent file system freeze/thaw
From: |
Jes Sorensen |
Subject: |
[Qemu-devel] Re: [PATCH 1/2] Add virtagent file system freeze/thaw |
Date: |
Wed, 02 Feb 2011 09:38:57 +0100 |
User-agent: |
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101209 Fedora/3.1.7-0.35.b3pre.fc14 Lightning/1.0b3pre Thunderbird/3.1.7 |
On 02/01/11 17:50, Michael Roth wrote:
> On 02/01/2011 04:58 AM, address@hidden wrote:
>> +enum vs_fsfreeze_status {
>> + FREEZE_ERROR = -1,
>> + FREEZE_THAWED = 0,
>> + FREEZE_INPROGRESS = 1,
>> + FREEZE_FROZEN = 2,
>> + FREEZE_THAWINPROGRESS = 3,
>> +};
>
> Any reason for vs_* vs. va_*?
Hmmmm let me see if I can find a good excuse for that typo :)
>> 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>
>
> Can probably clean these up a bit, I believe fcntl.h/errno.h/stat.h are
> already available at least.
Carry-over from writing the code outside of qemu. Would be much cleaner
than relying on the include everything and the kitchen sink in a global
header file, but thats how it is :(
>> +
>> + fsfreeze_status = FREEZE_INPROGRESS;
>> +
>> + entry = mount_list;
>
> I think as we start adding more and more stateful RPCs, free-floating
> state variables can start getting a bit hairy to keep track of.
> Eventually I'd like to have state information that only applies to a
> subset of RPCs consolidated into a single object. I wouldn't focus on
> this too much because I'd like to have an interface to do this in the
> future (mainly so they can state objects can register themselves and
> provide a reset() function that can be called when, for instance, an
> agent disconnects/reconnects), but in the meantime I think it would be
> more readable to have a global va_fsfreeze_state object to track freeze
> status and mount points.
Urgh, what do you mean by object here? I have to admit the word object
always makes me cringe.... I changed the variables to have the va_ prefix.
>> +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;
>> +}
>
> Hmm, you mentioned before that these freezes may be long-running
> jobs...do the ioctl()'s not return until completion? There is global
> timeout in virtagent, currently under a minute, to prevent a virtagent
> monitor command from hanging the monitor session, so if it's unlikely
> you'll fit in this window we'll need to work on something to better
> support these this kinds of situations.
I think 1 minute is fine, but we should probably look at something a
little more flexible for handling commands over the longer term. Maybe
have virtagent spawn threads for executing some commands?
> The 3 main approaches would be:
>
> 1) allow command-specific timeouts with values that are sane for the
> command in question, and potentially allow timeouts to be disabled
> 2) fork() long running jobs and provide a mechanism for them to provide
> asynchronous updates to us to we can query status
> 3) fork() long running jobs, have them provide status information
> elsewhere, and provide a polling function to check that status
>
> 3) would likely require something like writing status to a file and then
> provide a polling function to check it, which doesn't work here so
> that's probably out.
>
> I'd initially planned on doing 2) at some point, but I'm beginning to
> think 1) is the better approach, since qemu "opts in" on how long it's
> willing to hang for a particular command, so there's not really any
> surprises. At least not to qemu...users might get worried after a while,
> so there is a bit of a trade-off. But it's also more user-friendly....no
> need for polling or dealing with asynchronous updates to figure out when
> an RPC has actually finished. Seem reasonable?
I am not sure which is really the best solution. Basically we will need
to classify commands into two categories, so if you issue a certain type
of command, like agent_fsfreeze() (basically when the agent is in
FREEZE_FROZEN state) only status commands are allowed to execute in
parallel. Anything that tries to issue a write to the file system will
hang until agent_fsthaw is called. However it would be useful to be able
to call in for non-blocking status commands etc.
I'll post a v2 in a minute that addresses the issues pointed out by
Stefan and you. I think the threading/timeout aspect is something we
need to look at for the longer term.
Cheers,
Jes
- [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw, (continued)
- [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw, Jes . Sorensen, 2011/02/01
- Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw, Stefan Hajnoczi, 2011/02/01
- Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw, Jes Sorensen, 2011/02/01
- Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw, Stefan Hajnoczi, 2011/02/01
- Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw, Jes Sorensen, 2011/02/01
- Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw, Stefan Hajnoczi, 2011/02/01
- Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw, Michael Roth, 2011/02/01
[Qemu-devel] Re: [PATCH 1/2] Add virtagent file system freeze/thaw, Adam Litke, 2011/02/01
[Qemu-devel] Re: [PATCH 1/2] Add virtagent file system freeze/thaw, Michael Roth, 2011/02/01
- [Qemu-devel] Re: [PATCH 1/2] Add virtagent file system freeze/thaw,
Jes Sorensen <=
Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw, Stefan Hajnoczi, 2011/02/02
- Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw, Jes Sorensen, 2011/02/02
- Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw, Michael Roth, 2011/02/03
- Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw, Stefan Hajnoczi, 2011/02/04
- Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw, Michael Roth, 2011/02/04
- Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw, Stefan Hajnoczi, 2011/02/04
Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw, Jes Sorensen, 2011/02/04
Re: [Qemu-devel] [PATCH 1/2] Add virtagent file system freeze/thaw, Michael Roth, 2011/02/04
[Qemu-devel] [PATCH 2/2] Add monitor commands for fsfreeze support, Jes . Sorensen, 2011/02/01
Re: [Qemu-devel] [PATCH 0/2] virtagent - fsfreeze support, Vasiliy G Tolstov, 2011/02/01