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

["Jan Beulich" <JBeulich@suse.com>]

>>> On 19.10.17 at 17:06, <ian.jackson@eu.citrix.com> wrote:
> Jan Beulich writes ("Re: [PATCH] libxc: don't fail domain creation when 
> unpacking initrd fails"):
>> On 16.10.17 at 18:43, <ian.jackson@eu.citrix.com> wrote:
>> > I'm afraid I still find the patch less clear than it could be.
>> > The new semantics of xc_dom_ramdisk_check_size are awkward.  And
>> > looking at it briefly, I think it might be possible to try the unzip
>> > even if the size is too large.
>> 
>> I don't think so - xc_dom_ramdisk_check_size() returns 1
>> whenever decompressed size is above the limit. What I do
>> admit is that in the case compressed size is larger than
>> uncompressed size, with the boundary being in between, and
>> with decompression failing, we may accept something that's
>> above the limit. Not sure how bad that is though, as the limit
>> is pretty arbitrary anyway.
> 
> Conceptually what you are trying to do is have two alternative
> strategies.  Those two strategies have different limits.  So "the
> limit" is not a meaningful concept.
> 
>> > What you are really trying to do here is to pursue two strategies in
>> > parallel.  And ideally they would not be entangled.
>> 
>> I would have wanted to do things in sequence rather than in
>> parallel. I can't see how that could work though, in particular
>> when considering the case mentioned above (uncompressed size
>> smaller than compressed) - as the space allocation in the guest
>> can't be reverted, I need to allocate the larger of the two sizes
>> anyway.
> 
> I don't think it can work.  I think you uneed to pursue them in
> parallel and keep separate records, for each one, of whether we are
> still pursuing it or whether it has failed (and of course its
> necessary locals).

So before I do another pointless round of backporting (for the
change to be tested in the environment where it is needed),
does the below new function (with xc_dom_ramdisk_check_size()
dropped altogether) look any better to you?

Thanks, Jan

static int xc_dom_build_ramdisk(struct xc_dom_image *dom)
{
    size_t unziplen, ramdisklen;
    void *ramdiskmap;

    if ( !dom->ramdisk_seg.vstart )
        unziplen = xc_dom_check_gzip(dom->xch,
                                     dom->ramdisk_blob, dom->ramdisk_size);
    else
        unziplen = 0;

    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)
                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, &dom->ramdisk_seg, "ramdisk",
                              dom->ramdisk_seg.vstart, ramdisklen) != 0 )
        goto err;
    ramdiskmap = xc_dom_seg_to_ptr(dom, &dom->ramdisk_seg);
    if ( ramdiskmap == NULL )
    {
        DOMPRINTF("%s: xc_dom_seg_to_ptr(dom, &dom->ramdisk_seg) => NULL",
                  __FUNCTION__);
        goto err;
    }
    if ( unziplen )
    {
        if ( xc_dom_do_gunzip(dom->xch, dom->ramdisk_blob, dom->ramdisk_size,
                              ramdiskmap, unziplen) != -1 )
            return 0;
        if ( dom->ramdisk_size > ramdisklen )
            goto err;
    }

    /* Fall back to handing over the raw blob. */
    memcpy(ramdiskmap, dom->ramdisk_blob, dom->ramdisk_size);
    /* If an unzip attempt was made, the buffer may no longer be all zero. */
    if ( unziplen > dom->ramdisk_size )
        memset(ramdiskmap + dom->ramdisk_size, 0,
               unziplen - dom->ramdisk_size);

    return 0;

 err:
    return -1;
}



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel