All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Introduce dom0-min-space configuration option
@ 2010-07-12 18:07 Michal Novotny
  2010-07-13 18:02 ` Ian Jackson
  0 siblings, 1 reply; 17+ messages in thread
From: Michal Novotny @ 2010-07-12 18:07 UTC (permalink / raw)
  To: 'xen-devel@lists.xensource.com'

[-- Attachment #1: Type: text/plain, Size: 694 bytes --]

Hi,
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

Signed-off-by: Michal Novotny <minovotn@redhat.com>

-- 
Michal Novotny<minovotn@redhat.com>, RHCE
Virtualization Team (xen userspace), Red Hat


[-- Attachment #2: xen-introduce-dom0-min-space.patch --]
[-- Type: text/x-patch, Size: 2368 bytes --]

diff -r 9f08c4e82037 tools/examples/xend-config.sxp
--- a/tools/examples/xend-config.sxp	Mon Jul 12 18:39:18 2010 +0100
+++ b/tools/examples/xend-config.sxp	Mon Jul 12 22:05:56 2010 +0200
@@ -205,6 +205,11 @@
 # enable-dom0-ballooning below) and for xm mem-set when applied to dom0.
 (dom0-min-mem 196)
 
+# Dom0 will need at least specified amount of free space for
+# /var/lib/xen otherwise it will refuse to start up any new
+# guests. Value is in MB and setting it to 0 disables the check.
+(dom0-min-space 0)
+
 # Whether to enable auto-ballooning of dom0 to allow domUs to be created.
 # If enable-dom0-ballooning = no, dom0 will never balloon out.
 (enable-dom0-ballooning yes)
diff -r 9f08c4e82037 tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py	Mon Jul 12 18:39:18 2010 +0100
+++ b/tools/python/xen/xend/XendDomainInfo.py	Mon Jul 12 22:05:56 2010 +0200
@@ -2837,6 +2837,16 @@
 
         self._configureBootloader()
 
+        # Check whether dom0 does have enough free space to create the guest
+        stat = os.statvfs("/var/lib/xen")
+        free = int((stat.f_bsize * stat.f_bavail) / 1048576)
+        log.debug("Free space for Xen files on dom0: %s MiB" % free)
+        dom0_min_space = xoptions.instance().get_dom0_min_space()
+        if dom0_min_space > 0 and free < dom0_min_space:
+            raise XendError('Dom0 requires at least %s MiBs free space for '
+                            '/var/lib/xen but only %s MiBs is available.' %
+                            (dom0_min_space, free))
+
         try:
             self.image = image.create(self, self.info)
 
diff -r 9f08c4e82037 tools/python/xen/xend/XendOptions.py
--- a/tools/python/xen/xend/XendOptions.py	Mon Jul 12 18:39:18 2010 +0100
+++ b/tools/python/xen/xend/XendOptions.py	Mon Jul 12 22:05:56 2010 +0200
@@ -105,6 +105,8 @@
 
     dom0_min_mem_default = 0
 
+    dom0_min_space_default = 0
+
     reserved_memory_default = 0
 
     dom0_vcpus_default = 0
@@ -359,6 +361,9 @@
     def get_dom0_min_mem(self):
         return self.get_config_int('dom0-min-mem', self.dom0_min_mem_default)
 
+    def get_dom0_min_space(self):
+        return self.get_config_int('dom0-min-space', self.dom0_min_space_default)
+
     def get_enable_dom0_ballooning(self):
         enable_dom0_ballooning_default = 'yes'
         if self.get_dom0_min_mem() == 0:

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] Introduce dom0-min-space configuration option
  2010-07-12 18:07 [PATCH] Introduce dom0-min-space configuration option Michal Novotny
@ 2010-07-13 18:02 ` Ian Jackson
  2010-07-14 11:23   ` Michal Novotny
  0 siblings, 1 reply; 17+ messages in thread
From: Ian Jackson @ 2010-07-13 18:02 UTC (permalink / raw)
  To: Michal Novotny; +Cc: 'xen-devel@lists.xensource.com'

Michal Novotny writes ("[Xen-devel] [PATCH] 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.

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.

Ian.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] Introduce dom0-min-space configuration option
  2010-07-13 18:02 ` Ian Jackson
@ 2010-07-14 11:23   ` Michal Novotny
  2010-07-21  3:52     ` Michal Novotny
  2010-07-21 13:25     ` Ian Jackson
  0 siblings, 2 replies; 17+ messages in thread
From: Michal Novotny @ 2010-07-14 11:23 UTC (permalink / raw)
  To: Ian Jackson; +Cc: 'xen-devel@lists.xensource.com'

On 07/13/2010 08:02 PM, Ian Jackson wrote:
> Michal Novotny writes ("[Xen-devel] [PATCH] 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.
>>      
> 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 .
> 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.

> 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

-- 
Michal Novotny<minovotn@redhat.com>, RHCE
Virtualization Team (xen userspace), Red Hat

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] Introduce dom0-min-space configuration option
  2010-07-14 11:23   ` Michal Novotny
@ 2010-07-21  3:52     ` Michal Novotny
  2010-07-21 13:25     ` Ian Jackson
  1 sibling, 0 replies; 17+ messages in thread
From: Michal Novotny @ 2010-07-21  3:52 UTC (permalink / raw)
  To: Ian Jackson; +Cc: 'xen-devel@lists.xensource.com'

Ian,
what do you think about this?

Michal

On 07/14/2010 01:23 PM, Michal Novotny wrote:
> On 07/13/2010 08:02 PM, Ian Jackson wrote:
>> Michal Novotny writes ("[Xen-devel] [PATCH] 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.
>> 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 .
>> 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.
>
>> 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
>


-- 
Michal Novotny<minovotn@redhat.com>, RHCE
Virtualization Team (xen userspace), Red Hat

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] Introduce dom0-min-space configuration option
  2010-07-14 11:23   ` Michal Novotny
  2010-07-21  3:52     ` Michal Novotny
@ 2010-07-21 13:25     ` Ian Jackson
  2010-07-22  7:50       ` Michal Novotny
  1 sibling, 1 reply; 17+ messages in thread
From: Ian Jackson @ 2010-07-21 13:25 UTC (permalink / raw)
  To: Michal Novotny; +Cc: 'xen-devel@lists.xensource.com'

Michal Novotny writes ("Re: [Xen-devel] [PATCH] Introduce dom0-min-space configuration option"):
> 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.

>  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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] Introduce dom0-min-space configuration option
  2010-07-21 13:25     ` Ian Jackson
@ 2010-07-22  7:50       ` Michal Novotny
  2010-07-22  7:52         ` Michal Novotny
  2010-07-23 16:00         ` Ian Jackson
  0 siblings, 2 replies; 17+ messages in thread
From: Michal Novotny @ 2010-07-22  7:50 UTC (permalink / raw)
  To: Ian Jackson; +Cc: 'xen-devel@lists.xensource.com'

On 07/21/2010 03:25 PM, Ian Jackson wrote:
> Michal Novotny writes ("Re: [Xen-devel] [PATCH] Introduce dom0-min-space configuration option"):
>    
>> 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.

>>   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.

> 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.

Michal

-- 
Michal Novotny<minovotn@redhat.com>, RHCE
Virtualization Team (xen userspace), Red Hat

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] Introduce dom0-min-space configuration option
  2010-07-22  7:50       ` Michal Novotny
@ 2010-07-22  7:52         ` Michal Novotny
  2010-07-23 16:00         ` Ian Jackson
  1 sibling, 0 replies; 17+ messages in thread
From: Michal Novotny @ 2010-07-22  7:52 UTC (permalink / raw)
  To: xen-devel, Ian Jackson

On 07/22/2010 09:50 AM, Michal Novotny wrote:
> On 07/21/2010 03:25 PM, Ian Jackson wrote:
>> Michal Novotny writes ("Re: [Xen-devel] [PATCH] Introduce 
>> dom0-min-space configuration option"):
>>> 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.
>
>>>   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.
>
>> 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.

Oh, sorry, I meant x86_64 RHEL-6 host.

Michal


-- 
Michal Novotny<minovotn@redhat.com>, RHCE
Virtualization Team (xen userspace), Red Hat

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] Introduce dom0-min-space configuration option
  2010-07-22  7:50       ` Michal Novotny
  2010-07-22  7:52         ` Michal Novotny
@ 2010-07-23 16:00         ` Ian Jackson
  2010-07-26  6:55           ` Michal Novotny
  1 sibling, 1 reply; 17+ messages in thread
From: Ian Jackson @ 2010-07-23 16:00 UTC (permalink / raw)
  To: Michal Novotny; +Cc: 'xen-devel@lists.xensource.com', Ian Jackson

Michal Novotny writes ("Re: [Xen-devel] [PATCH] Introduce dom0-min-space configuration option"):
> 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.

> Why do you think my patch is entirely wrong?

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

Ian.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] Introduce dom0-min-space configuration option
  2010-07-23 16:00         ` Ian Jackson
@ 2010-07-26  6:55           ` Michal Novotny
  2010-07-26  9:59             ` Paolo Bonzini
  0 siblings, 1 reply; 17+ messages in thread
From: Michal Novotny @ 2010-07-26  6:55 UTC (permalink / raw)
  To: Ian Jackson; +Cc: 'xen-devel@lists.xensource.com'

On 07/23/2010 06:00 PM, Ian Jackson wrote:
> Michal Novotny writes ("Re: [Xen-devel] [PATCH] Introduce dom0-min-space configuration option"):
>    
>> 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.
>    

Well, if the kernel is extracted partially could it get decompressed 
well? I don't think so since there's a data stream missing.

>    
>> Why do you think my patch is entirely wrong?
>>      
> Because you are treating the symptoms but not addressing the cause.
>
> Ian.
>    

 From what I know this is caused by insufficient disk space and 
therefore the kernel and/or initrd are extracted just partially so 
there's no valid end of the input data stream.

I think that making sure that the data will be extracted fully and 
successfully from the image file is fixing the root cause. 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?

Michal

-- 
Michal Novotny<minovotn@redhat.com>, RHCE
Virtualization Team (xen userspace), Red Hat

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] Introduce dom0-min-space configuration option
  2010-07-26  6:55           ` Michal Novotny
@ 2010-07-26  9:59             ` Paolo Bonzini
  2010-07-26 10:36               ` Michal Novotny
  0 siblings, 1 reply; 17+ messages in thread
From: Paolo Bonzini @ 2010-07-26  9:59 UTC (permalink / raw)
  To: Michal Novotny; +Cc: 'xen-devel@lists.xensource.com', Ian Jackson

On 07/26/2010 08:55 AM, Michal Novotny wrote:
> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] Introduce dom0-min-space configuration option
  2010-07-26  9:59             ` Paolo Bonzini
@ 2010-07-26 10:36               ` Michal Novotny
  2010-07-26 11:18                 ` Paolo Bonzini
  0 siblings, 1 reply; 17+ messages in thread
From: Michal Novotny @ 2010-07-26 10:36 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: 'xen-devel@lists.xensource.com', Ian Jackson

On 07/26/2010 11:59 AM, Paolo Bonzini wrote:
> On 07/26/2010 08:55 AM, Michal Novotny wrote:
>> 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

Paolo, that's correct but the issue here is that we don't know until we 
extract it from image file. But if an error occurs within the extraction 
we could write something like "pygrub: No space left on device" but for 
virt-manager and libvirt-based tools it will return an error "Boot 
loader didn't return any data!" so it would make other confusion to the 
user since user won't be able to know what's going on. Maybe the check 
for this message (or any other message) coming from pygrub could help 
and this message could be raised in the body of XendError/VmError which 
would make user know what the problem is at least.

Michal

-- 
Michal Novotny<minovotn@redhat.com>, RHCE
Virtualization Team (xen userspace), Red Hat

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] Introduce dom0-min-space configuration option
  2010-07-26 10:36               ` Michal Novotny
@ 2010-07-26 11:18                 ` Paolo Bonzini
  2010-07-26 11:21                   ` Michal Novotny
  0 siblings, 1 reply; 17+ messages in thread
From: Paolo Bonzini @ 2010-07-26 11:18 UTC (permalink / raw)
  To: Michal Novotny; +Cc: 'xen-devel@lists.xensource.com', Ian Jackson

On 07/26/2010 12:36 PM, Michal Novotny wrote:
> On 07/26/2010 11:59 AM, Paolo Bonzini wrote:
>> On 07/26/2010 08:55 AM, Michal Novotny wrote:
>>> 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
>
> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] Introduce dom0-min-space configuration option
  2010-07-26 11:18                 ` Paolo Bonzini
@ 2010-07-26 11:21                   ` Michal Novotny
  2010-07-26 11:31                     ` Paolo Bonzini
  0 siblings, 1 reply; 17+ messages in thread
From: Michal Novotny @ 2010-07-26 11:21 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: 'xen-devel@lists.xensource.com', Ian Jackson

On 07/26/2010 01:18 PM, Paolo Bonzini wrote:
> On 07/26/2010 12:36 PM, Michal Novotny wrote:
>> On 07/26/2010 11:59 AM, Paolo Bonzini wrote:
>>> On 07/26/2010 08:55 AM, Michal Novotny wrote:
>>>> 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
>>
>> 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
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.

Michal

-- 
Michal Novotny<minovotn@redhat.com>, RHCE
Virtualization Team (xen userspace), Red Hat

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] Introduce dom0-min-space configuration option
  2010-07-26 11:21                   ` Michal Novotny
@ 2010-07-26 11:31                     ` Paolo Bonzini
  2010-07-26 11:48                       ` Michal Novotny
  0 siblings, 1 reply; 17+ messages in thread
From: Paolo Bonzini @ 2010-07-26 11:31 UTC (permalink / raw)
  To: Michal Novotny; +Cc: 'xen-devel@lists.xensource.com', Ian Jackson

On 07/26/2010 01:21 PM, Michal Novotny wrote:
> On 07/26/2010 01:18 PM, Paolo Bonzini wrote:
>> On 07/26/2010 12:36 PM, Michal Novotny wrote:
>>> On 07/26/2010 11:59 AM, Paolo Bonzini wrote:
>>>> On 07/26/2010 08:55 AM, Michal Novotny wrote:
>>>>> 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
>>>
>>> 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?
>
> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] Introduce dom0-min-space configuration option
  2010-07-26 11:31                     ` Paolo Bonzini
@ 2010-07-26 11:48                       ` Michal Novotny
  2010-07-26 12:23                         ` Paolo Bonzini
  0 siblings, 1 reply; 17+ messages in thread
From: Michal Novotny @ 2010-07-26 11:48 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: 'xen-devel@lists.xensource.com', Ian Jackson

On 07/26/2010 01:31 PM, Paolo Bonzini wrote:
> On 07/26/2010 01:21 PM, Michal Novotny wrote:
>> On 07/26/2010 01:18 PM, Paolo Bonzini wrote:
>>> On 07/26/2010 12:36 PM, Michal Novotny wrote:
>>>> On 07/26/2010 11:59 AM, Paolo Bonzini wrote:
>>>>> On 07/26/2010 08:55 AM, Michal Novotny wrote:
>>>>>> 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
>>>>
>>>> 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?
>>
>> 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
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?

Michal


-- 
Michal Novotny<minovotn@redhat.com>, RHCE
Virtualization Team (xen userspace), Red Hat

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] Introduce dom0-min-space configuration option
  2010-07-26 11:48                       ` Michal Novotny
@ 2010-07-26 12:23                         ` Paolo Bonzini
  2010-07-26 12:48                           ` Michal Novotny
  0 siblings, 1 reply; 17+ messages in thread
From: Paolo Bonzini @ 2010-07-26 12:23 UTC (permalink / raw)
  To: Michal Novotny; +Cc: 'xen-devel@lists.xensource.com', Ian Jackson

On 07/26/2010 01:48 PM, Michal Novotny wrote:
>>
> 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?

Yes, definitely.

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

Paolo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] Introduce dom0-min-space configuration option
  2010-07-26 12:23                         ` Paolo Bonzini
@ 2010-07-26 12:48                           ` Michal Novotny
  0 siblings, 0 replies; 17+ messages in thread
From: Michal Novotny @ 2010-07-26 12:48 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: 'xen-devel@lists.xensource.com', Ian Jackson

On 07/26/2010 02:23 PM, Paolo Bonzini wrote:
> On 07/26/2010 01:48 PM, Michal Novotny wrote:
>>>
>> 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?
>
> Yes, definitely.
>
> Option 1 wouldn't work, initrd and kernel files have all sort of sizes.
>
> Paolo
Ok, this way it makes sense, however when setting dom0-min-space to 100 
(MiB) there's no such kernel/initrd that big but I'm not saying this is 
bad. Somewhat better error reporting will be the right way to go after all.

Michal

-- 
Michal Novotny<minovotn@redhat.com>, RHCE
Virtualization Team (xen userspace), Red Hat

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2010-07-26 12:48 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-12 18:07 [PATCH] Introduce dom0-min-space configuration option Michal Novotny
2010-07-13 18:02 ` Ian Jackson
2010-07-14 11:23   ` Michal Novotny
2010-07-21  3:52     ` Michal Novotny
2010-07-21 13:25     ` Ian Jackson
2010-07-22  7:50       ` Michal Novotny
2010-07-22  7:52         ` Michal Novotny
2010-07-23 16:00         ` Ian Jackson
2010-07-26  6:55           ` Michal Novotny
2010-07-26  9:59             ` Paolo Bonzini
2010-07-26 10:36               ` Michal Novotny
2010-07-26 11:18                 ` Paolo Bonzini
2010-07-26 11:21                   ` Michal Novotny
2010-07-26 11:31                     ` Paolo Bonzini
2010-07-26 11:48                       ` Michal Novotny
2010-07-26 12:23                         ` Paolo Bonzini
2010-07-26 12:48                           ` Michal Novotny

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.