Stefano Stabellini
2011-Jan-18 17:17 UTC
[Xen-devel] [PATCH 1/3] libxl: add 2 consoles to stubdoms for save/restore
Add two "special" PV consoles to stubdoms that are going to be used
to send and receive the qemu-xen save files on save/restore.
Rename the save file on resume so that it doesn''t collide with the name
of the next save file.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
diff -r 59396addc940 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c Fri Jan 14 14:26:11 2011 +0000
+++ b/tools/libxl/libxl.c Tue Jan 18 17:09:56 2011 +0000
@@ -750,7 +750,7 @@ int libxl_primary_console_exec(libxl_ctx
{
uint32_t stubdomid = libxl_get_stubdom_id(ctx, domid_vm);
if (stubdomid)
- return libxl_console_exec(ctx, stubdomid, 1, LIBXL_CONSTYPE_PV);
+ return libxl_console_exec(ctx, stubdomid, 3, LIBXL_CONSTYPE_PV);
else {
if (libxl__domain_is_hvm(ctx, domid_vm))
return libxl_console_exec(ctx, domid_vm, 0, LIBXL_CONSTYPE_SERIAL);
diff -r 59396addc940 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c Fri Jan 14 14:26:11 2011 +0000
+++ b/tools/libxl/libxl_create.c Tue Jan 18 17:09:56 2011 +0000
@@ -250,8 +250,12 @@ static int domain_restore(libxl_ctx *ctx
dm_info->saved_state = NULL;
if (info->hvm) {
+ char buf[100];
+ snprintf(buf, sizeof(buf), "/var/lib/xen/qemu-save.%d",
domid);
ret = asprintf(&dm_info->saved_state,
- "/var/lib/xen/qemu-save.%d", domid);
+ "/var/lib/xen/qemu-restore.%d", domid);
+ if (ret >= 0)
+ ret = rename(buf, dm_info->saved_state);
ret = (ret < 0) ? ERROR_FAIL : 0;
}
diff -r 59396addc940 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c Fri Jan 14 14:26:11 2011 +0000
+++ b/tools/libxl/libxl_dm.c Tue Jan 18 17:09:56 2011 +0000
@@ -430,7 +430,7 @@ static int libxl_create_stubdom(libxl_ct
libxl__device_model_starting **starting_r)
{
libxl__gc gc = LIBXL_INIT_GC(ctx);
- int i, num_console = 1, ret;
+ int i, num_console = 3, ret;
libxl_device_console *console;
libxl_domain_create_info c_info;
libxl_domain_build_info b_info;
@@ -533,15 +533,28 @@ retry_transaction:
console[i].devid = i;
console[i].consback = LIBXL_CONSBACK_IOEMU;
console[i].domid = domid;
- if (!i) {
- char *filename;
- char *name = libxl__sprintf(&gc, "qemu-dm-%s",
libxl_domid_to_name(ctx, info->domid));
- libxl_create_logfile(ctx, name, &filename);
- console[i].output = libxl__sprintf(&gc, "file:%s",
filename);
- console[i].build_state = &state;
- free(filename);
- } else
- console[i].output = "pty";
+ switch (i) {
+ case 0:
+ {
+ char *filename;
+ char *name = libxl__sprintf(&gc,
"qemu-dm-%s", libxl_domid_to_name(ctx, info->domid));
+ libxl_create_logfile(ctx, name, &filename);
+ console[i].output = libxl__sprintf(&gc,
"file:%s", filename);
+ console[i].build_state = &state;
+ free(filename);
+ }
+ break;
+ case 1:
+ console[i].output = libxl__sprintf(&gc,
"file:/var/lib/xen/qemu-save.%d", info->domid);
+ break;
+ case 2:
+ if (info->saved_state)
+ console[i].output = libxl__sprintf(&gc,
"filerw:%s", info->saved_state);
+ break;
+ default:
+ console[i].output = "pty";
+ break;
+ }
ret = libxl_device_console_add(ctx, domid, &console[i]);
if (ret)
goto out_free;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jan-18 17:26 UTC
Re: [Xen-devel] [PATCH 1/3] libxl: add 2 consoles to stubdoms for save/restore
On Tue, 2011-01-18 at 17:17 +0000, Stefano Stabellini wrote:> Rename the save file on resume so that it doesn''t collide with the > name of the next save file.What is the underlying issue here? Why does it matter if a save uses the same filename as a previous resume? Or is the issue that you can accidentally resume an image twice? Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2011-Jan-18 17:29 UTC
Re: [Xen-devel] [PATCH 1/3] libxl: add 2 consoles to stubdoms for save/restore
On Tue, 18 Jan 2011, Ian Campbell wrote:> On Tue, 2011-01-18 at 17:17 +0000, Stefano Stabellini wrote: > > Rename the save file on resume so that it doesn''t collide with the > > name of the next save file. > > What is the underlying issue here? Why does it matter if a save uses the > same filename as a previous resume? Or is the issue that you can > accidentally resume an image twice?The issue is that the console backend is going to open both the resume file and the next save file when it starts, so they cannot have the same name. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jan-18 17:31 UTC
Re: [Xen-devel] [PATCH 1/3] libxl: add 2 consoles to stubdoms for save/restore
On Tue, 2011-01-18 at 17:29 +0000, Stefano Stabellini wrote:> On Tue, 18 Jan 2011, Ian Campbell wrote: > > On Tue, 2011-01-18 at 17:17 +0000, Stefano Stabellini wrote: > > > Rename the save file on resume so that it doesn''t collide with the > > > name of the next save file. > > > > What is the underlying issue here? Why does it matter if a save uses the > > same filename as a previous resume? Or is the issue that you can > > accidentally resume an image twice? > > The issue is that the console backend is going to open both the resume > file and the next save file when it starts, so they cannot have the same > name.Makes sense. I think it is worth writing this in the checking comment. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2011-Jan-18 17:41 UTC
Re: [Xen-devel] [PATCH 1/3] libxl: add 2 consoles to stubdoms for save/restore
On Tue, 18 Jan 2011, Ian Campbell wrote:> On Tue, 2011-01-18 at 17:29 +0000, Stefano Stabellini wrote: > > On Tue, 18 Jan 2011, Ian Campbell wrote: > > > On Tue, 2011-01-18 at 17:17 +0000, Stefano Stabellini wrote: > > > > Rename the save file on resume so that it doesn''t collide with the > > > > name of the next save file. > > > > > > What is the underlying issue here? Why does it matter if a save uses the > > > same filename as a previous resume? Or is the issue that you can > > > accidentally resume an image twice? > > > > The issue is that the console backend is going to open both the resume > > file and the next save file when it starts, so they cannot have the same > > name. > > Makes sense. > > I think it is worth writing this in the checking comment.Add two "special" PV consoles to stubdoms that are going to be used to send and receive the qemu-xen save files on save/restore. Rename the save file on resume so that it doesn''t collide with the name of the next save file (that is opened by the console backend as soon as it starts so it cannot be the same as the resume state file name). Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> diff -r 59396addc940 tools/libxl/libxl.c --- a/tools/libxl/libxl.c Fri Jan 14 14:26:11 2011 +0000 +++ b/tools/libxl/libxl.c Tue Jan 18 17:09:56 2011 +0000 @@ -750,7 +750,7 @@ int libxl_primary_console_exec(libxl_ctx { uint32_t stubdomid = libxl_get_stubdom_id(ctx, domid_vm); if (stubdomid) - return libxl_console_exec(ctx, stubdomid, 1, LIBXL_CONSTYPE_PV); + return libxl_console_exec(ctx, stubdomid, 3, LIBXL_CONSTYPE_PV); else { if (libxl__domain_is_hvm(ctx, domid_vm)) return libxl_console_exec(ctx, domid_vm, 0, LIBXL_CONSTYPE_SERIAL); diff -r 59396addc940 tools/libxl/libxl_create.c --- a/tools/libxl/libxl_create.c Fri Jan 14 14:26:11 2011 +0000 +++ b/tools/libxl/libxl_create.c Tue Jan 18 17:09:56 2011 +0000 @@ -250,8 +250,12 @@ static int domain_restore(libxl_ctx *ctx dm_info->saved_state = NULL; if (info->hvm) { + char buf[100]; + snprintf(buf, sizeof(buf), "/var/lib/xen/qemu-save.%d", domid); ret = asprintf(&dm_info->saved_state, - "/var/lib/xen/qemu-save.%d", domid); + "/var/lib/xen/qemu-restore.%d", domid); + if (ret >= 0) + ret = rename(buf, dm_info->saved_state); ret = (ret < 0) ? ERROR_FAIL : 0; } diff -r 59396addc940 tools/libxl/libxl_dm.c --- a/tools/libxl/libxl_dm.c Fri Jan 14 14:26:11 2011 +0000 +++ b/tools/libxl/libxl_dm.c Tue Jan 18 17:09:56 2011 +0000 @@ -430,7 +430,7 @@ static int libxl_create_stubdom(libxl_ct libxl__device_model_starting **starting_r) { libxl__gc gc = LIBXL_INIT_GC(ctx); - int i, num_console = 1, ret; + int i, num_console = 3, ret; libxl_device_console *console; libxl_domain_create_info c_info; libxl_domain_build_info b_info; @@ -533,15 +533,28 @@ retry_transaction: console[i].devid = i; console[i].consback = LIBXL_CONSBACK_IOEMU; console[i].domid = domid; - if (!i) { - char *filename; - char *name = libxl__sprintf(&gc, "qemu-dm-%s", libxl_domid_to_name(ctx, info->domid)); - libxl_create_logfile(ctx, name, &filename); - console[i].output = libxl__sprintf(&gc, "file:%s", filename); - console[i].build_state = &state; - free(filename); - } else - console[i].output = "pty"; + switch (i) { + case 0: + { + char *filename; + char *name = libxl__sprintf(&gc, "qemu-dm-%s", libxl_domid_to_name(ctx, info->domid)); + libxl_create_logfile(ctx, name, &filename); + console[i].output = libxl__sprintf(&gc, "file:%s", filename); + console[i].build_state = &state; + free(filename); + } + break; + case 1: + console[i].output = libxl__sprintf(&gc, "file:/var/lib/xen/qemu-save.%d", info->domid); + break; + case 2: + if (info->saved_state) + console[i].output = libxl__sprintf(&gc, "filerw:%s", info->saved_state); + break; + default: + console[i].output = "pty"; + break; + } ret = libxl_device_console_add(ctx, domid, &console[i]); if (ret) goto out_free; _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jan-19 11:12 UTC
Re: [Xen-devel] [PATCH 1/3] libxl: add 2 consoles to stubdoms for save/restore
On Tue, 2011-01-18 at 17:17 +0000, Stefano Stabellini wrote:> Add two "special" PV consoles to stubdoms that are going to be used > to send and receive the qemu-xen save files on save/restore. > > Rename the save file on resume so that it doesn''t collide with the name > of the next save file. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > diff -r 59396addc940 tools/libxl/libxl.c > --- a/tools/libxl/libxl.c Fri Jan 14 14:26:11 2011 +0000 > +++ b/tools/libxl/libxl.c Tue Jan 18 17:09:56 2011 +0000 > @@ -750,7 +750,7 @@ int libxl_primary_console_exec(libxl_ctx > { > uint32_t stubdomid = libxl_get_stubdom_id(ctx, domid_vm); > if (stubdomid) > - return libxl_console_exec(ctx, stubdomid, 1, LIBXL_CONSTYPE_PV); > + return libxl_console_exec(ctx, stubdomid, 3, LIBXL_CONSTYPE_PV);#define STUBDOM_CONSOLE_xxx or an enum to give a symbolic name to each of the 3-4 console numbers.> else { > if (libxl__domain_is_hvm(ctx, domid_vm)) > return libxl_console_exec(ctx, domid_vm, 0, LIBXL_CONSTYPE_SERIAL); > diff -r 59396addc940 tools/libxl/libxl_create.c > --- a/tools/libxl/libxl_create.c Fri Jan 14 14:26:11 2011 +0000 > +++ b/tools/libxl/libxl_create.c Tue Jan 18 17:09:56 2011 +0000 > @@ -250,8 +250,12 @@ static int domain_restore(libxl_ctx *ctx > > dm_info->saved_state = NULL; > if (info->hvm) { > + char buf[100]; > + snprintf(buf, sizeof(buf), "/var/lib/xen/qemu-save.%d", domid); > ret = asprintf(&dm_info->saved_state, > - "/var/lib/xen/qemu-save.%d", domid); > + "/var/lib/xen/qemu-restore.%d", domid);These two strings end up in a couple of places, probably worth either a #define or libxl__domain_dm_{save,restore}_path(...). char buf[100] is pretty ugly too. Either use the relevant libxl_Xprintf or a number much smaller than 100 since domid is bounded -- i.e. strlen("/var/lib/xen/qemu-save.") + strlen("65536") (+ 1?). 32 seems to be more than sufficient. [...]> + case 1: > + console[i].output = libxl__sprintf(&gc, "file:/var/lib/xen/qemu-save.%d", info->domid); > + break; > + case 2: > + if (info->saved_state) > + console[i].output = libxl__sprintf(&gc, "filerw:%s", info->saved_state);These "file:" and "filerw:" prefixed things are interpreted by qemu within the stub domain? Since you need to introduce filerw why not instead introduce something like pvconsole:{0,1,2,3} (or /dev/consoleN etc)? Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2011-Jan-19 11:53 UTC
Re: [Xen-devel] [PATCH 1/3] libxl: add 2 consoles to stubdoms for save/restore
On Wed, 19 Jan 2011, Ian Campbell wrote:> On Tue, 2011-01-18 at 17:17 +0000, Stefano Stabellini wrote: > > Add two "special" PV consoles to stubdoms that are going to be used > > to send and receive the qemu-xen save files on save/restore. > > > > Rename the save file on resume so that it doesn''t collide with the name > > of the next save file. > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > > > > diff -r 59396addc940 tools/libxl/libxl.c > > --- a/tools/libxl/libxl.c Fri Jan 14 14:26:11 2011 +0000 > > +++ b/tools/libxl/libxl.c Tue Jan 18 17:09:56 2011 +0000 > > @@ -750,7 +750,7 @@ int libxl_primary_console_exec(libxl_ctx > > { > > uint32_t stubdomid = libxl_get_stubdom_id(ctx, domid_vm); > > if (stubdomid) > > - return libxl_console_exec(ctx, stubdomid, 1, LIBXL_CONSTYPE_PV); > > + return libxl_console_exec(ctx, stubdomid, 3, LIBXL_CONSTYPE_PV); > > #define STUBDOM_CONSOLE_xxx or an enum to give a symbolic name to each > of the 3-4 console numbers.good idea> > > else { > > if (libxl__domain_is_hvm(ctx, domid_vm)) > > return libxl_console_exec(ctx, domid_vm, 0, LIBXL_CONSTYPE_SERIAL); > > diff -r 59396addc940 tools/libxl/libxl_create.c > > --- a/tools/libxl/libxl_create.c Fri Jan 14 14:26:11 2011 +0000 > > +++ b/tools/libxl/libxl_create.c Tue Jan 18 17:09:56 2011 +0000 > > @@ -250,8 +250,12 @@ static int domain_restore(libxl_ctx *ctx > > > > dm_info->saved_state = NULL; > > if (info->hvm) { > > + char buf[100]; > > + snprintf(buf, sizeof(buf), "/var/lib/xen/qemu-save.%d", domid); > > ret = asprintf(&dm_info->saved_state, > > - "/var/lib/xen/qemu-save.%d", domid); > > + "/var/lib/xen/qemu-restore.%d", domid); > > These two strings end up in a couple of places, probably worth either a > #define or libxl__domain_dm_{save,restore}_path(...). >considering that the restore string comes from libxc I''ll go with an #define> char buf[100] is pretty ugly too. Either use the relevant libxl_Xprintf > or a number much smaller than 100 since domid is bounded -- i.e. > strlen("/var/lib/xen/qemu-save.") + strlen("65536") (+ 1?). 32 seems to > be more than sufficient. >I am going to remove it and modify libxc to save to a different filename in the first place> [...] > > + case 1: > > + console[i].output = libxl__sprintf(&gc, "file:/var/lib/xen/qemu-save.%d", info->domid); > > + break; > > + case 2: > > + if (info->saved_state) > > + console[i].output = libxl__sprintf(&gc, "filerw:%s", info->saved_state); > > These "file:" and "filerw:" prefixed things are interpreted by qemu > within the stub domain?No, they are interpreted by the console backend in dom0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel