Re: [PATCH v5 05/21] libxl: Handle Linux stubdomain specific QEMU options.

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

Jason Andryuk writes ("[PATCH v5 05/21] libxl: Handle Linux stubdomain specific 
QEMU options."):
> From: Eric Shelton <eshelton@xxxxxxxxx>
> 
> 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 <eshelton@xxxxxxxxx>
> 
> Simon:
>  * fix disk path
>  * fix cdrom path and "format"
> 
> Signed-off-by: Simon Gaiser <simon@xxxxxxxxxxxxxxxxxxxxxx>
> [drop Qubes-specific parts]
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>

Nice work all.

Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

This is despite me spotting three tiny style nits:

> @@ -1312,7 +1316,7 @@ static int libxl__build_device_model_args_new(libxl__gc 
> *gc,
>          }
>  
>          flexarray_append(dm_args, vncarg);
> -    } else
> +    } else if (!is_stubdom)
>          /*
>           * Ensure that by default no vnc server is created.
>           */

While you are here it would be nice to regularise the { }.  (libxl
CODING_STYLE says that all branches of an if should have { }, if any
of them do.)

> @@ -1974,8 +2006,10 @@ static int libxl__build_device_model_args(libxl__gc 
> *gc,
>                                                    args, envs,
>                                                    state);
>      case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> -        assert(dm_state_fd != NULL);
> -        assert(*dm_state_fd < 0);
> +        if 
> (!libxl_defbool_val(guest_config->b_info.device_model_stubdomain)) {
> +            assert(dm_state_fd != NULL);
> +            assert(*dm_state_fd < 0);
> +     }

This } seems to be misindented ?

>      if (guest_config->b_info.u.hvm.serial)
>          num_console++;
> +    else if (guest_config->b_info.u.hvm.serial_list) {
> +        char **serial = guest_config->b_info.u.hvm.serial_list;
> +        while (*(serial++))
> +            num_console++;
> +    }

You should add the { } areound the if block too.

Ian.