Re: [Xen-devel] [PATCH] Do not read files at once in pygrub

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2012-05-22 at 08:37 +0100, Miroslav Rezanina wrote:
> If guest kernel or ramdisk image is too large to fit in the dom0 memory, it 
> causes pygrub crash with "out of memory" error. 
> This patch reads kernel/ramdisk file in 1 MB blocks so prevent exhausting 
> whole memory.

Thanks, we've had a similar patch from Michael Young (CCd) too, see:
<alpine.DEB.2.00.1205170029550.26049@xxxxxxxxxxxxxxxx>.

They look functionally pretty similar, one big difference is that
Michael limits the size of the cfg file as well, which seems wise.

Looks like his handles errors on the os.write too?

I do like splitting the read loop out into a function as you've done
though.

> 
> Signed-off-by: Miroslav Rezanina <mrezanin@xxxxxxxxxx>
> 
> Patch:
> --
> diff -r 238900a4ed22 tools/pygrub/src/pygrub
> --- a/tools/pygrub/src/pygrub Mon May 21 12:03:32 2012 +0200
> +++ b/tools/pygrub/src/pygrub Tue May 22 11:32:28 2012 +0200
> @@ -697,6 +697,20 @@
>      def usage():
>          print >> sys.stderr, "Usage: %s [-q|--quiet] [-i|--interactive] 
> [-n|--not-really] [--output=] [--kernel=] [--ramdisk=] [--args=] [--entry=] 
> [--output-directory=] [--output-format=sxp|simple|simple0] <image>" 
> %(sys.argv[0],)
>  
> +    def copy_from_image(fs, file_to_copy, tmp_prefix):
> +        (tfd, rv) = tempfile.mkstemp(prefix=tmp_prefix, dir="/var/lib/xen")
> +        file_handle = fs.open_file(file_to_copy)
> +        readsize = 0
> +
> +        while True:
> +            data = file_handle.read(1048576,readsize)
> +            if len(data) == 0:
> +                os.close(tfd)
> +                return rv
> +
>   +          readsize += len(data)
> +            os.write(tfd,data)          
> +
>      try:
>          opts, args = getopt.gnu_getopt(sys.argv[1:], 'qinh::',
>                                     ["quiet", "interactive", "not-really", 
> "help", 
> @@ -824,21 +838,15 @@
>      if not_really:
>          bootcfg["kernel"] = "<kernel:%s>" % chosencfg["kernel"]
>      else:
> -        data = fs.open_file(chosencfg["kernel"]).read()
> -        (tfd, bootcfg["kernel"]) = tempfile.mkstemp(prefix="boot_kernel.",
> -                                                    dir=output_directory)
> -        os.write(tfd, data)
> -        os.close(tfd)
> +        bootcfg["kernel"] = copy_from_image(fs,chosencfg["kernel"],
> +                                            "boot_kernel.")
>  
>      if chosencfg["ramdisk"]:
>          if not_really:
>              bootcfg["ramdisk"] = "<ramdisk:%s>" % chosencfg["ramdisk"]
>          else:
> -            data = fs.open_file(chosencfg["ramdisk"],).read()
> -            (tfd, bootcfg["ramdisk"]) = tempfile.mkstemp(
> -                prefix="boot_ramdisk.", dir=output_directory)
> -            os.write(tfd, data)
> -            os.close(tfd)
> +            bootcfg["ramdisk"] = copy_from_image(fs, chosencfg["ramdisk",
> +                                                 "boot_ramdisk.")
>      else:
>          initrd = None
>  
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel



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