| Message ID | 1458677199-10301-1-git-send-email-firas.azar@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 22/03/16 20:06, Firas Azar wrote:
> The pygrub command option "isconfig" is broken and obsolete. This patch removes it since the alternate options "-l -n" provide the same functionality.
Broken how?
Are you sure that no existing software is using that option?
~Andrew
Andrew: Currently if we use pygrub --isconfig <image>, there is a good chance that this command could fail with an "out of memory" error. This can happen especially if the image file in question is at least a few gigabytes in size and the dom0_mem setting is relatively small. So even if any software used this option it is likely it would fail. This problem is due to the parser using "readlines" function in GrubConf.py, which attempts to load the entire file into memory. The advantage of "-l -n" option is that it exercises the "real" code path, which boots the PV guests. Regards --Firas On 03/22/2016 04:22 PM, Andrew Cooper wrote: > On 22/03/16 20:06, Firas Azar wrote: >> The pygrub command option "isconfig" is broken and obsolete. This patch removes it since the alternate options "-l -n" provide the same functionality. > Broken how? > > Are you sure that no existing software is using that option? > > ~Andrew
On 03/22/2016 04:46 PM, Firas Azar wrote: > Andrew: > > Currently if we use pygrub --isconfig <image>, there is a good chance > that this command could fail with an "out of memory" error. This can > happen especially if the image file in question is at least a few > gigabytes in size and the dom0_mem setting is relatively small. So > even if any software used this option it is likely it would fail. This > problem is due to the parser using "readlines" function in > GrubConf.py, which attempts to load the entire file into memory. FWIW, the same problem is observed on unstable. Running this option results in very high disk IO rate and eventually OOM killer wakes up and whacks pygrub. So chances are this has been broken for a while since I believe Firas is working with 4.4. I haven't tried anything other than unstable though. -boris > > The advantage of "-l -n" option is that it exercises the "real" code > path, which boots the PV guests. > > Regards > > --Firas > > On 03/22/2016 04:22 PM, Andrew Cooper wrote: >> On 22/03/16 20:06, Firas Azar wrote: >>> The pygrub command option "isconfig" is broken and obsolete. This >>> patch removes it since the alternate options "-l -n" provide the >>> same functionality. >> Broken how? >> >> Are you sure that no existing software is using that option? >> >> ~Andrew > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub index 40f9584..82d4f01 100755 --- a/tools/pygrub/src/pygrub +++ b/tools/pygrub/src/pygrub @@ -776,10 +776,11 @@ if __name__ == "__main__": try: opts, args = getopt.gnu_getopt(sys.argv[1:], 'qilnh::', - ["quiet", "interactive", "list-entries", "not-really", "help", - "output=", "output-format=", "output-directory=", "offset=", - "entry=", "kernel=", - "ramdisk=", "args=", "isconfig", "debug"]) + ["quiet", "interactive", "list-entries", + "not-really", "help", "output=", + "output-format=", "output-directory=", + "offset=", "entry=", "kernel=", + "ramdisk=", "args=", "debug"]) except getopt.GetoptError: usage() sys.exit(1) @@ -793,7 +794,6 @@ if __name__ == "__main__": entry = None interactive = True list_entries = False - isconfig = False part_offs = None debug = False not_really = False @@ -838,8 +838,6 @@ if __name__ == "__main__": entry = a # specifying the entry to boot implies non-interactive interactive = False - elif o in ("--isconfig",): - isconfig = True elif o in ("--debug",): debug = True elif o in ("--output-format",): @@ -871,15 +869,6 @@ if __name__ == "__main__": else: fd = os.open(output, os.O_WRONLY) - # debug - if isconfig: - chosencfg = run_grub(file, entry, fs, incfg["args"]) - print " kernel: %s" % chosencfg["kernel"] - if chosencfg["ramdisk"]: - print " initrd: %s" % chosencfg["ramdisk"] - print " args: %s" % chosencfg["args"] - sys.exit(0) - # if boot filesystem is set then pass to fsimage.open bootfsargs = '"%s"' % incfg["args"] bootfsgroup = re.findall('zfs-bootfs=(.*?)[\s\,\"]', bootfsargs)
The pygrub command option "isconfig" is broken and obsolete. This patch removes it since the alternate options "-l -n" provide the same functionality. Signed-off-by: Firas Azar <firas.azar@oracle.com> --- tools/pygrub/src/pygrub | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-)