Re: [Xen-devel] [PATCH v2] libxc: don't fail domain creation when unpacking initrd fails

[Wei Liu]

On Mon, Dec 04, 2017 at 05:46:41AM -0700, Jan Beulich wrote:
> At least Linux kernels have been able to work with gzip-ed initrd for
> quite some time; initrd compressed with other methods aren't even being
> attempted to unpack. Furthermore the unzip-ing routine used here isn't
> capable of dealing with various forms of concatenated files, each of
> which was gzip-ed separately (it is this particular case which has been
> the source of observed VM creation failures).
> 
> Hence, if unpacking fails, simply hand the the compressed blob to the
> guest as is.
> 
> Signed-off-by: Jan Beulich 

The code looks OK to me. It requires rebase though.

Wei.

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

Re: [Xen-devel] [PATCH v2] libxc: don't fail domain creation when unpacking initrd fails

[Jan Beulich]

>>> On 23.01.18 at 11:55,  wrote:
> On Mon, Dec 04, 2017 at 05:46:41AM -0700, Jan Beulich wrote:
>> At least Linux kernels have been able to work with gzip-ed initrd for
>> quite some time; initrd compressed with other methods aren't even being
>> attempted to unpack. Furthermore the unzip-ing routine used here isn't
>> capable of dealing with various forms of concatenated files, each of
>> which was gzip-ed separately (it is this particular case which has been
>> the source of observed VM creation failures).
>> 
>> Hence, if unpacking fails, simply hand the the compressed blob to the
>> guest as is.
> 
> Sadly this will have to be rebased on top of staging,
> xc_dom_build_ramdisk was renamed to xc_dom_build_module.

That shouldn't be difficult to do, but makes little sense until I know
I don't need to make other changes. Note for how long the patch
has been pending.

>> @@ -1020,11 +1015,18 @@ static int xc_dom_build_ramdisk(struct x
>>  if ( unziplen )
>>  {
>>  if ( xc_dom_do_gunzip(dom->xch, dom->ramdisk_blob, 
>> dom->ramdisk_size,
>> -  ramdiskmap, ramdisklen) == -1 )
>> +  ramdiskmap, unziplen) != -1 )
>> +return 0;
>> +if ( dom->ramdisk_size > ramdisklen )
> 
> AFAICT this would mean that the non-gzipped ramdisk would be bigger
> than the gzipped one?

Yes (an at least theoretical possibility).

> The code LGTM.

Thanks, Jan


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

Re: [Xen-devel] [PATCH v2] libxc: don't fail domain creation when unpacking initrd fails

[Roger Pau Monné]

On Mon, Dec 04, 2017 at 05:46:41AM -0700, Jan Beulich wrote:
> At least Linux kernels have been able to work with gzip-ed initrd for
> quite some time; initrd compressed with other methods aren't even being
> attempted to unpack. Furthermore the unzip-ing routine used here isn't
> capable of dealing with various forms of concatenated files, each of
> which was gzip-ed separately (it is this particular case which has been
> the source of observed VM creation failures).
> 
> Hence, if unpacking fails, simply hand the the compressed blob to the
> guest as is.

Sadly this will have to be rebased on top of staging,
xc_dom_build_ramdisk was renamed to xc_dom_build_module.

> Signed-off-by: Jan Beulich 
> ---
> v2: Almost full re-work, hopefully better meeting Ian's taste.
> 
> --- a/tools/libxc/include/xc_dom.h
> +++ b/tools/libxc/include/xc_dom.h
> @@ -291,7 +291,6 @@ int xc_dom_mem_init(struct xc_dom_image
>  int xc_dom_kernel_check_size(struct xc_dom_image *dom, size_t sz);
>  int xc_dom_kernel_max_size(struct xc_dom_image *dom, size_t sz);
>  
> -int xc_dom_ramdisk_check_size(struct xc_dom_image *dom, size_t sz);
>  int xc_dom_ramdisk_max_size(struct xc_dom_image *dom, size_t sz);
>  
>  int xc_dom_devicetree_max_size(struct xc_dom_image *dom, size_t sz);
> --- a/tools/libxc/xc_dom_core.c
> +++ b/tools/libxc/xc_dom_core.c
> @@ -314,22 +314,6 @@ int xc_dom_kernel_check_size(struct xc_d
>  return 0;
>  }
>  
> -int xc_dom_ramdisk_check_size(struct xc_dom_image *dom, size_t sz)
> -{
> -/* No limit */
> -if ( !dom->max_ramdisk_size )
> -return 0;
> -
> -if ( sz > dom->max_ramdisk_size )
> -{
> -xc_dom_panic(dom->xch, XC_INVALID_KERNEL,
> - "ramdisk image too large");
> -return 1;
> -}
> -
> -return 0;
> -}
> -
>  /*  
> */
>  /* read files, copy memory blocks, with transparent gunzip  
> */
>  
> @@ -996,16 +980,27 @@ static int xc_dom_build_ramdisk(struct x
>  void *ramdiskmap;
>  
>  if ( !dom->ramdisk_seg.vstart )
> -{
>  unziplen = xc_dom_check_gzip(dom->xch,
>   dom->ramdisk_blob, dom->ramdisk_size);
> -if ( xc_dom_ramdisk_check_size(dom, unziplen) != 0 )
> -unziplen = 0;
> -}
>  else
>  unziplen = 0;
>  
> -ramdisklen = unziplen ? unziplen : dom->ramdisk_size;
> +ramdisklen = max(unziplen, dom->ramdisk_size);
> +if ( dom->max_ramdisk_size )
> +{
> +if ( unziplen && ramdisklen > dom->max_ramdisk_size )
> +{
> +ramdisklen = min(unziplen, dom->ramdisk_size);
> +if ( unziplen > ramdisklen)
 ^ missing space

> +unziplen = 0;
> +}
> +if ( ramdisklen > dom->max_ramdisk_size )
> +{
> +xc_dom_panic(dom->xch, XC_INVALID_KERNEL,
> + "ramdisk image too large");
> +goto err;
> +}
> +}
>  
>  if ( xc_dom_alloc_segment(dom, >ramdisk_seg, "ramdisk",
>dom->ramdisk_seg.vstart, ramdisklen) != 0 )
> @@ -1020,11 +1015,18 @@ static int xc_dom_build_ramdisk(struct x
>  if ( unziplen )
>  {
>  if ( xc_dom_do_gunzip(dom->xch, dom->ramdisk_blob, dom->ramdisk_size,
> -  ramdiskmap, ramdisklen) == -1 )
> +  ramdiskmap, unziplen) != -1 )
> +return 0;
> +if ( dom->ramdisk_size > ramdisklen )

AFAICT this would mean that the non-gzipped ramdisk would be bigger
than the gzipped one?

The code LGTM.

Thanks, Roger.

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