Re: [Xen-devel] [RFC PATCH 03/17] libxl: Handle Linux stubdomain specific QEMU options.

2018-08-02 Thread Jason Andryuk
On Wed, Aug 1, 2018 at 11:55 AM Marek Marczykowski-Górecki
 wrote:
>
> On Wed, Aug 01, 2018 at 10:25:22AM -0400, Jason Andryuk wrote:
> > On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
> >  wrote:
> > > From: Eric Shelton 
> > >
> > > This patch creates an appropriate command line for the QEMU instance
> > > running in a Linux-based stubdomain.
> > >
> > > NOTE: a number of items are not currently implemented for Linux-based
> > > stubdomains, such as:
> > > - save/restore
> > > - QMP socket
> > > - graphics output (e.g., VNC)
> > >
> > > Signed-off-by: Eric Shelton 
> > >
> > > Simon:
> > >  * fix disk path
> > >  * fix cdrom path and "format"
> > >  * pass downscript for network interfaces
> > > ---
> > 
> > > @@ -1099,10 +1118,21 @@ static int 
> > > libxl__build_device_model_args_new(libxl__gc *gc,
> > >  return ERROR_INVAL;
> > >  }
> > >  if (b_info->u.hvm.serial) {
> > > -flexarray_vappend(dm_args,
> > > -  "-serial", b_info->u.hvm.serial, NULL);
> > > +if (is_stubdom) {
> > > +flexarray_vappend(dm_args,
> > > +  "-serial",
> > > +  GCSPRINTF("/dev/hvc%d", 
> > > STUBDOM_CONSOLE_SERIAL),
> > > +  NULL);
> > > +} else {
> > > +flexarray_vappend(dm_args,
> > > +  "-serial", b_info->u.hvm.serial, 
> > > NULL);
> > > +}
> > >  } else if (b_info->u.hvm.serial_list) {
> > >  char **p;
> > > +if (is_stubdom) {
> > > +flexarray_vappend(dm_args,
> > > +  "-serial", "/dev/hvc1", NULL);
> >
> > Should this also be GCSPRINTF("/dev/hvc%d", STUBDOM_CONSOLE_SERIAL)
> > instead of hardcoding hvc1?
>
> Yes. Anyway,  multiple serial consoles are incompatible with
> stubdomain anyway - should it error out if serial_list have multiple
> elements? Or silently ignore others?

I think error-ing out with a suitable message for multiple elements
would avoid confusion.  Silently ignoring would leave users wondering
why the extra consoles didn't show up.

Multiple serials could be supported by dynamically allocating more
consoles to stubdom, but it's probably not worth the effort.

> > > +}
> > >  for (p = b_info->u.hvm.serial_list;
> > >   *p;
> > >   p++) {
> >
> > 
> >
> > > @@ -1550,8 +1584,8 @@ static int 
> > > libxl__build_device_model_args_new(libxl__gc *gc,
> > >
> > >  if (disks[i].is_cdrom) {
> > >  drive = libxl__sprintf(gc,
> > > - 
> > > "if=ide,index=%d,readonly=on,media=cdrom,id=ide-%i",
> > > - disk, dev_number);
> > > + "if=ide,readonly=on,media=cdrom,id=ide-%i",
> > > + dev_number);
> >
> > What is the impact of dropping index?
>
> Leftover from Qubes-specific part of this patch (non-cdrom disks are
> converted to SCSI for readonly support, dropping index avoids hitting
> IDE limitation of 4 disks). I'll remove this chunk.

Ah, okay.

Regards,
Jason

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [RFC PATCH 03/17] libxl: Handle Linux stubdomain specific QEMU options.

2018-08-01 Thread Marek Marczykowski-Górecki
On Wed, Aug 01, 2018 at 10:25:22AM -0400, Jason Andryuk wrote:
> On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
>  wrote:
> > From: Eric Shelton 
> >
> > This patch creates an appropriate command line for the QEMU instance
> > running in a Linux-based stubdomain.
> >
> > NOTE: a number of items are not currently implemented for Linux-based
> > stubdomains, such as:
> > - save/restore
> > - QMP socket
> > - graphics output (e.g., VNC)
> >
> > Signed-off-by: Eric Shelton 
> >
> > Simon:
> >  * fix disk path
> >  * fix cdrom path and "format"
> >  * pass downscript for network interfaces
> > ---
> 
> > @@ -1099,10 +1118,21 @@ static int 
> > libxl__build_device_model_args_new(libxl__gc *gc,
> >  return ERROR_INVAL;
> >  }
> >  if (b_info->u.hvm.serial) {
> > -flexarray_vappend(dm_args,
> > -  "-serial", b_info->u.hvm.serial, NULL);
> > +if (is_stubdom) {
> > +flexarray_vappend(dm_args,
> > +  "-serial",
> > +  GCSPRINTF("/dev/hvc%d", 
> > STUBDOM_CONSOLE_SERIAL),
> > +  NULL);
> > +} else {
> > +flexarray_vappend(dm_args,
> > +  "-serial", b_info->u.hvm.serial, 
> > NULL);
> > +}
> >  } else if (b_info->u.hvm.serial_list) {
> >  char **p;
> > +if (is_stubdom) {
> > +flexarray_vappend(dm_args,
> > +  "-serial", "/dev/hvc1", NULL);
> 
> Should this also be GCSPRINTF("/dev/hvc%d", STUBDOM_CONSOLE_SERIAL)
> instead of hardcoding hvc1?

Yes. Anyway,  multiple serial consoles are incompatible with
stubdomain anyway - should it error out if serial_list have multiple
elements? Or silently ignore others?

> > +}
> >  for (p = b_info->u.hvm.serial_list;
> >   *p;
> >   p++) {
> 
> 
> 
> > @@ -1550,8 +1584,8 @@ static int 
> > libxl__build_device_model_args_new(libxl__gc *gc,
> >
> >  if (disks[i].is_cdrom) {
> >  drive = libxl__sprintf(gc,
> > - 
> > "if=ide,index=%d,readonly=on,media=cdrom,id=ide-%i",
> > - disk, dev_number);
> > + "if=ide,readonly=on,media=cdrom,id=ide-%i",
> > + dev_number);
> 
> What is the impact of dropping index?

Leftover from Qubes-specific part of this patch (non-cdrom disks are
converted to SCSI for readonly support, dropping index avoids hitting
IDE limitation of 4 disks). I'll remove this chunk.

> >  if (target_path)
> >  drive = libxl__sprintf(gc, "%s,file=%s,format=%s",
> 
> Regards,
> Jason

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [RFC PATCH 03/17] libxl: Handle Linux stubdomain specific QEMU options.

2018-08-01 Thread Jason Andryuk
On Mon, Jul 30, 2018 at 11:56 PM, Marek Marczykowski-Górecki
 wrote:
> From: Eric Shelton 
>
> This patch creates an appropriate command line for the QEMU instance
> running in a Linux-based stubdomain.
>
> NOTE: a number of items are not currently implemented for Linux-based
> stubdomains, such as:
> - save/restore
> - QMP socket
> - graphics output (e.g., VNC)
>
> Signed-off-by: Eric Shelton 
>
> Simon:
>  * fix disk path
>  * fix cdrom path and "format"
>  * pass downscript for network interfaces
> ---

> @@ -1099,10 +1118,21 @@ static int 
> libxl__build_device_model_args_new(libxl__gc *gc,
>  return ERROR_INVAL;
>  }
>  if (b_info->u.hvm.serial) {
> -flexarray_vappend(dm_args,
> -  "-serial", b_info->u.hvm.serial, NULL);
> +if (is_stubdom) {
> +flexarray_vappend(dm_args,
> +  "-serial",
> +  GCSPRINTF("/dev/hvc%d", 
> STUBDOM_CONSOLE_SERIAL),
> +  NULL);
> +} else {
> +flexarray_vappend(dm_args,
> +  "-serial", b_info->u.hvm.serial, NULL);
> +}
>  } else if (b_info->u.hvm.serial_list) {
>  char **p;
> +if (is_stubdom) {
> +flexarray_vappend(dm_args,
> +  "-serial", "/dev/hvc1", NULL);

Should this also be GCSPRINTF("/dev/hvc%d", STUBDOM_CONSOLE_SERIAL)
instead of hardcoding hvc1?

> +}
>  for (p = b_info->u.hvm.serial_list;
>   *p;
>   p++) {



> @@ -1550,8 +1584,8 @@ static int libxl__build_device_model_args_new(libxl__gc 
> *gc,
>
>  if (disks[i].is_cdrom) {
>  drive = libxl__sprintf(gc,
> - "if=ide,index=%d,readonly=on,media=cdrom,id=ide-%i",
> - disk, dev_number);
> + "if=ide,readonly=on,media=cdrom,id=ide-%i",
> + dev_number);

What is the impact of dropping index?

>  if (target_path)
>  drive = libxl__sprintf(gc, "%s,file=%s,format=%s",

Regards,
Jason

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [RFC PATCH 03/17] libxl: Handle Linux stubdomain specific QEMU options.

2018-07-30 Thread Marek Marczykowski-Górecki
From: Eric Shelton 

This patch creates an appropriate command line for the QEMU instance
running in a Linux-based stubdomain.

NOTE: a number of items are not currently implemented for Linux-based
stubdomains, such as:
- save/restore
- QMP socket
- graphics output (e.g., VNC)

Signed-off-by: Eric Shelton 

Simon:
 * fix disk path
 * fix cdrom path and "format"
 * pass downscript for network interfaces
---
 tools/libxl/libxl_dm.c | 106 --
 1 file changed, 72 insertions(+), 34 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 3b0b58e..b38c1d2 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -25,9 +25,24 @@
 #include 
 #include 
 
-static const char *libxl_tapif_script(libxl__gc *gc)
+static const char *libxl_tapif_script(libxl__gc *gc,
+  const libxl_domain_build_info *info)
 {
 #if defined(__linux__) || defined(__FreeBSD__)
+if (info->stubdomain_version == LIBXL_STUBDOMAIN_VERSION_LINUX)
+return libxl__sprintf(gc, "/etc/qemu-ifup");
+return libxl__strdup(gc, "no");
+#else
+return GCSPRINTF("%s/qemu-ifup", libxl__xen_script_dir_path());
+#endif
+}
+
+static const char *libxl_tapif_downscript(libxl__gc *gc,
+  const libxl_domain_build_info *info)
+{
+#if defined(__linux__) || defined(__FreeBSD__)
+if (info->stubdomain_version == LIBXL_STUBDOMAIN_VERSION_LINUX)
+return libxl__sprintf(gc, "/etc/qemu-ifdown");
 return libxl__strdup(gc, "no");
 #else
 return GCSPRINTF("%s/qemu-ifup", libxl__xen_script_dir_path());
@@ -616,8 +631,8 @@ static int libxl__build_device_model_args_old(libxl__gc *gc,
   "tap,vlan=%d,ifname=%s,bridge=%s,"
   "script=%s,downscript=%s",
   nics[i].devid, ifname, nics[i].bridge,
-  libxl_tapif_script(gc),
-  libxl_tapif_script(gc)),
+  libxl_tapif_script(gc, b_info),
+  libxl_tapif_downscript(gc, b_info)),
   NULL);
 ioemu_nics++;
 }
@@ -933,6 +948,7 @@ static int libxl__build_device_model_args_new(libxl__gc *gc,
 const char *path, *chardev;
 char *user = NULL;
 struct passwd *user_base, user_pwbuf;
+bool is_stubdom = libxl_defbool_val(b_info->device_model_stubdomain);
 
 dm_args = flexarray_make(gc, 16, 1);
 dm_envs = flexarray_make(gc, 16, 1);
@@ -943,24 +959,27 @@ static int libxl__build_device_model_args_new(libxl__gc 
*gc,
   "-xen-domid",
   GCSPRINTF("%d", guest_domid), NULL);
 
-flexarray_append(dm_args, "-chardev");
-flexarray_append(dm_args,
- GCSPRINTF("socket,id=libxl-cmd,"
-"path=%s/qmp-libxl-%d,server,nowait",
-libxl__run_dir_path(), guest_domid));
+/* There is currently no way to access the QMP socket in the stubdom */
+if (!is_stubdom) {
+flexarray_append(dm_args, "-chardev");
+flexarray_append(dm_args,
+ GCSPRINTF("socket,id=libxl-cmd,"
+"path=%s/qmp-libxl-%d,server,nowait",
+libxl__run_dir_path(), guest_domid));
 
-flexarray_append(dm_args, "-no-shutdown");
-flexarray_append(dm_args, "-mon");
-flexarray_append(dm_args, "chardev=libxl-cmd,mode=control");
+flexarray_append(dm_args, "-no-shutdown");
+flexarray_append(dm_args, "-mon");
+flexarray_append(dm_args, "chardev=libxl-cmd,mode=control");
 
-flexarray_append(dm_args, "-chardev");
-flexarray_append(dm_args,
- GCSPRINTF("socket,id=libxenstat-cmd,"
-"path=%s/qmp-libxenstat-%d,server,nowait",
-libxl__run_dir_path(), guest_domid));
+flexarray_append(dm_args, "-chardev");
+flexarray_append(dm_args,
+ GCSPRINTF("socket,id=libxenstat-cmd,"
+
"path=%s/qmp-libxenstat-%d,server,nowait",
+libxl__run_dir_path(), guest_domid));
 
-flexarray_append(dm_args, "-mon");
-flexarray_append(dm_args, "chardev=libxenstat-cmd,mode=control");
+flexarray_append(dm_args, "-mon");
+flexarray_append(dm_args, "chardev=libxenstat-cmd,mode=control");
+}
 
 for (i = 0; i < guest_config->num_channels; i++) {
 connection = guest_config->channels[i].connection;
@@ -1004,7 +1023,7 @@ static int libxl__build_device_model_args_new(libxl__gc 
*gc,
 flexarray_vappend(dm_args, "-name", c_info->name, NULL);
 }
 
-if (vnc) {
+if (vnc && !is_stubdom) {