Introduce dom0-min-space configuration option

This is the patch to introduce configuration option called dom0-min-space since there were some issues with data inflation because of invalid input data stream for zlib decompression. The issue occured because of insufficient free space on the dom0 so this patch checks the free available space for /var/lib/xen and refuses to start up any guests when the space is below specified value. Setting up the value to 0 disables the check which preserves the behaviour before this patch applied and this is the default value for this option.

Michal Novotny writes ("[Xen-devel] [PATCH] Introduce dom0-min-space configuration option"):

Quoted 10 lines

This is the patch to introduce configuration option called dom0-min-space since there were some issues with data inflation because of invalid input data stream for zlib decompression. The issue occured because of insufficient free space on the dom0 so this patch checks the free available space for /var/lib/xen and refuses to start up any guests when the space is below specified value. Setting up the value to 0 disables the check which preserves the behaviour before this patch applied and this is the default value for this option.

Thanks for the patch, but I'm not sure I entirely follow.

What "issues with data inflation because of invalid input data stream for zlib decompression" were there and how do they relate to lack of space on /var/lib/xen ?

Is this just for the situation where the dom0 filesystem hasn't enough space to contain the uncompressed version of the domU kernel and initrd which are to be loaded ?

Your patch just raises an error. Why does the situation in which the patch is needed not already raise an appropriate error ? Perhaps it would be better to arrange that it does.

Quoted 10 lines · Ian Jackson

Michal Novotny writes ("[Xen-devel] [PATCH] Introduce dom0-min-space configuration option"):

Quoted 9 lines

This is the patch to introduce configuration option called dom0-min-space since there were some issues with data inflation because of invalid input data stream for zlib decompression. The issue occured because of insufficient free space on the dom0 so this patch checks the free available space for /var/lib/xen and refuses to start up any guests when the space is below specified value. Setting up the value to 0 disables the check which preserves the behaviour before this patch applied and this is the default value for this option.

Quoted 5 lines

Thanks for the patch, but I'm not sure I entirely follow.

What "issues with data inflation because of invalid input data stream for zlib decompression" were there and how do they relate to lack of space on /var/lib/xen ?

Ian, the problem is when pygrub extracts the vmlinuz and initrd for PV guests but there's insufficient space on the dom0, there's no error message but the error is being raised from Xend itself, libxc to be precise since there's the zStream inflation code but since the input data are not valid (i.e. they're just partial, let's say only 50 KiB was extracted to /var/lib/xen since after those 50 KiBs the dom0 had no space available) the zStream (zlib decompression) fails with Z_BUF_ERROR and then it outputs annoying and nothing saying message to standalone users (non-developers) to doesn't do investigation on their own. This patch would prevent going into those issues since there would be always at least specified amount of free space available for PV images to be extracted to /var/lib/xen .

Quoted 3 lines

Is this just for the situation where the dom0 filesystem hasn't enough space to contain the uncompressed version of the domU kernel and initrd which are to be loaded ?

Well, this is for compressed version being extracted from the guest image files. This is being saved to /var/lib/xen and if there's not enough space to save it the deflation fails with -5 (Z_BUF_ERROR) which says nothing to standalone users.

Quoted 3 lines

Your patch just raises an error. Why does the situation in which the patch is needed not already raise an appropriate error ? Perhaps it would be better to arrange that it does.

Well, the reason is that there should be multiple reasons that could make the decompression (gunzip) fail. Also, having the dom0 with no space could cause some other (non-Xen) related errors so having this option could prevent user running into those issues - mainly if the user's saving images to the default location, i.e. /var/lib/xen/images, he/she can get running out of free space on dom0 very easily.

Michal

Ian, the problem is when pygrub extracts the vmlinuz and initrd for PV guests but there's insufficient space on the dom0, there's no error message but the error is being raised from Xend itself, [...]

libxc to be precise since there's the zStream inflation code but
since the input data are not valid (i.e. they're just partial,

Michal Novotny writes ("Re: [Xen-devel] [PATCH] Introduce dom0-min-space configuration option"):

Quoted 4 lines

Ian, the problem is when pygrub extracts the vmlinuz and initrd for PV guests but there's insufficient space on the dom0, there's no error message but the error is being raised from Xend itself, [...]

So the correct fix is to make sure the error propagation works correctly.

Quoted 3 lines

libxc to be precise since there's the zStream inflation code but
since the input data are not valid (i.e. they're just partial,

That suggests that not only is the error being bungled somewhere, it's being outright ignored and something tries to decompress the truncated file.

These bugs should be fixed.

But I think your patch is entirely wrong, I'm afraid.

Ian.

Quoted 7 lines · Ian Jackson

Michal Novotny writes ("Re: [Xen-devel] [PATCH] Introduce dom0-min-space configuration option"):

Quoted 3 lines

Ian, the problem is when pygrub extracts the vmlinuz and initrd for PV guests but there's insufficient space on the dom0, there's no error message but the error is being raised from Xend itself, [...]

So the correct fix is to make sure the error propagation works correctly.

Well, the patching in the all the parts of the code may be better but there may be many issues caused there by dom0 being out of space and not all of them have to be xen related - nevertheless users can think they're xen related until they find out the dom0 is out of space and therefore although Xen may *not* be responsible they can blame Xen for that.

Quoted 5 lines

Quoted 2 lines

libxc to be precise since there's the zStream inflation code but
since the input data are not valid (i.e. they're just partial,

That suggests that not only is the error being bungled somewhere, it's being outright ignored and something tries to decompress the truncated file.

Well, there's no other way to fix it than to implement a check for enough space on dom0 for extracting vmlinuz and initrd files from the image file. If it's partially extracted it's understandable that the data are not valid input for zStream processing and therefore it runs into Z_BUF_ERROR (-5) during the process.

Quoted 5 lines

These bugs should be fixed.

But I think your patch is entirely wrong, I'm afraid.

Ian.

Why do you think my patch is entirely wrong? If you're talking about functionality it's been tested on x86_64 RHEL-6 guest with upstream Xen installed.

Well, there's no other way to fix it than to implement a check for enough space on dom0 for extracting vmlinuz and initrd files from the image file. If it's partially extracted it's understandable that the data are not valid input for zStream processing and therefore it runs into Z_BUF_ERROR (-5) during the process.

Why do you think my patch is entirely wrong?

Michal Novotny writes ("Re: [Xen-devel] [PATCH] Introduce dom0-min-space configuration option"):

Quoted 6 lines

Well, there's no other way to fix it than to implement a check for enough space on dom0 for extracting vmlinuz and initrd files from the image file. If it's partially extracted it's understandable that the data are not valid input for zStream processing and therefore it runs into Z_BUF_ERROR (-5) during the process.

No, it is not understandable that the system should continue to try to execute a partially-extracted kernel. This needs to be fixed.

Quoted 3 lines

Why do you think my patch is entirely wrong?

Because you are treating the symptoms but not addressing the cause.

Ian.

Or do you think that we should alter the message in gunzip function to say that there's an error in data stream (premature end of data stream) and that user should check for enough space on dom0?

Quoted 3 lines · Michal Novotny

Or do you think that we should alter the message in gunzip function to say that there's an error in data stream (premature end of data stream) and that user should check for enough space on dom0?

No, the gunzip function (in libxc, if I understood the context) should not even be called if pygrub could not write the file. Instead, it should print something like

pygrub: No space left on device

and exit. There's absolutely no error checking here:

    data = fs.open_file(chosencfg["kernel"]).read()
    (tfd, bootcfg["kernel"]) = tempfile.mkstemp(prefix="boot_kernel.",
        dir="/var/run/xend/boot")
    os.write(tfd, data)
    os.close(tfd)

(and likewise for initrd) and that's the bug.

Paolo

Quoted 19 lines · Paolo Bonzini

Quoted 3 lines · Michal Novotny

Or do you think that we should alter the message in gunzip function to say that there's an error in data stream (premature end of data stream) and that user should check for enough space on dom0?

No, the gunzip function (in libxc, if I understood the context) should not even be called if pygrub could not write the file. Instead, it should print something like

pygrub: No space left on device

and exit. There's absolutely no error checking here:

data = fs.open_file(chosencfg["kernel"]).read()
(tfd, bootcfg["kernel"]) = tempfile.mkstemp(prefix="boot_kernel.", dir="/var/run/xend/boot")
os.write(tfd, data)
os.close(tfd)

Quoted 3 lines

(and likewise for initrd) and that's the bug.

Paolo

Paolo, that's correct but the issue here is that we don't know until we extract it from image file.

Quoted 22 lines · Michal Novotny

Quoted 21 lines · Paolo Bonzini

Quoted 3 lines · Michal Novotny

Or do you think that we should alter the message in gunzip function to say that there's an error in data stream (premature end of data stream) and that user should check for enough space on dom0?

No, the gunzip function (in libxc, if I understood the context) should not even be called if pygrub could not write the file. Instead, it should print something like

pygrub: No space left on device

and exit. There's absolutely no error checking here:

data = fs.open_file(chosencfg["kernel"]).read()
(tfd, bootcfg["kernel"]) = tempfile.mkstemp(prefix="boot_kernel.", dir="/var/run/xend/boot")
os.write(tfd, data)
os.close(tfd)

(and likewise for initrd) and that's the bug.

Quoted 4 lines

Quoted 1 line

Paolo

Paolo, that's correct but the issue here is that we don't know until we extract it from image file.

os.write would return -1 and set errno to ENOSPC (besides, any other errno should get the same treatment), no?

Paolo

Quoted 29 lines · Paolo Bonzini

Quoted 27 lines · Michal Novotny

Quoted 20 lines · Paolo Bonzini

Quoted 4 lines · Michal Novotny

Or do you think that we should alter the message in gunzip function to say that there's an error in data stream (premature end of data stream)
and that user should check for enough space on dom0?

No, the gunzip function (in libxc, if I understood the context) should not even be called if pygrub could not write the file. Instead, it should print something like

pygrub: No space left on device

and exit. There's absolutely no error checking here:

data = fs.open_file(chosencfg["kernel"]).read()
(tfd, bootcfg["kernel"]) = tempfile.mkstemp(prefix="boot_kernel.", dir="/var/run/xend/boot")
os.write(tfd, data)
os.close(tfd)

Quoted 3 lines

(and likewise for initrd) and that's the bug.

Paolo

Paolo, that's correct but the issue here is that we don't know until we extract it from image file.

Quoted 2 lines

os.write would return -1 and set errno to ENOSPC (besides, any other errno should get the same treatment), no?

Maybe, I need to check the docs first but you're most likely right about this. Nevertheless this is the pygrub code AFAIK so some patch for xend would be necessary as well, otherwise libvirt-based tools would complain with "Boot loader didn't return any data!" message.

Quoted 33 lines · Michal Novotny

Quoted 31 lines · Paolo Bonzini

Quoted 21 lines · Michal Novotny

Quoted 20 lines · Paolo Bonzini

Quoted 4 lines · Michal Novotny

Or do you think that we should alter the message in gunzip function to say that there's an error in data stream (premature end of data stream)
and that user should check for enough space on dom0?

No, the gunzip function (in libxc, if I understood the context) should not even be called if pygrub could not write the file. Instead, it should print something like

pygrub: No space left on device

and exit. There's absolutely no error checking here:

data = fs.open_file(chosencfg["kernel"]).read()
(tfd, bootcfg["kernel"]) = tempfile.mkstemp(prefix="boot_kernel.", dir="/var/run/xend/boot")
os.write(tfd, data)
os.close(tfd)

Quoted 6 lines

Quoted 3 lines

(and likewise for initrd) and that's the bug.

Paolo

Paolo, that's correct but the issue here is that we don't know until we extract it from image file.

os.write would return -1 and set errno to ENOSPC (besides, any other errno should get the same treatment), no?

Quoted 4 lines

Maybe, I need to check the docs first but you're most likely right about this. Nevertheless this is the pygrub code AFAIK so some patch for xend would be necessary as well, otherwise libvirt-based tools would complain with "Boot loader didn't return any data!" message.

That's the least of the problems, if a sensible error message is present too. I agree with Ian, let's first fix the main cause. Then we can see what the fallout is.

Paolo

Quoted 1 line

Well, the root cause is insufficient disk space since without insufficient disk space issue we won't be running into those issues. In fact there are 2 points of view on this one:

1) implement a check for enough disk space (this is what I did)
2) implement the proper error message saying there is not enough space
to extract it (ENOSPC) - this is what you and Ian suggest

I am not telling any of those options is bad, nevertheless it's based just on the point of view. So, is option 2 better and worth implementing rather than limiting the domain run only to case with enough space on dom0?

Quoted 5 lines · Michal Novotny

Quoted 1 line

Well, the root cause is insufficient disk space since without insufficient disk space issue we won't be running into those issues. In fact there are 2 points of view on this one:

Quoted 8 lines

1) implement a check for enough disk space (this is what I did)
2) implement the proper error message saying there is not enough space
to extract it (ENOSPC) - this is what you and Ian suggest

I am not telling any of those options is bad, nevertheless it's based just on the point of view. So, is option 2 better and worth implementing rather than limiting the domain run only to case with enough space on dom0?

Yes, definitely.

Option 1 wouldn't work, initrd and kernel files have all sort of sizes.

Paolo