xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] pygrub: remove isconfig option
@ 2016-03-22 20:06 Firas Azar
  2016-03-22 20:22 ` Andrew Cooper
  0 siblings, 1 reply; 4+ messages in thread
From: Firas Azar @ 2016-03-22 20:06 UTC (permalink / raw)
  To: xen-devel; +Cc: firas.azar

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(-)

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)
-- 
1.9.1


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

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

* Re: [PATCH] pygrub: remove isconfig option
  2016-03-22 20:06 [PATCH] pygrub: remove isconfig option Firas Azar
@ 2016-03-22 20:22 ` Andrew Cooper
  2016-03-22 20:46   ` Firas Azar
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Cooper @ 2016-03-22 20:22 UTC (permalink / raw)
  To: Firas Azar, xen-devel

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

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

* Re: [PATCH] pygrub: remove isconfig option
  2016-03-22 20:22 ` Andrew Cooper
@ 2016-03-22 20:46   ` Firas Azar
  2016-03-22 20:54     ` Boris Ostrovsky
  0 siblings, 1 reply; 4+ messages in thread
From: Firas Azar @ 2016-03-22 20:46 UTC (permalink / raw)
  To: Andrew Cooper, xen-devel

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


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

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

* Re: [PATCH] pygrub: remove isconfig option
  2016-03-22 20:46   ` Firas Azar
@ 2016-03-22 20:54     ` Boris Ostrovsky
  0 siblings, 0 replies; 4+ messages in thread
From: Boris Ostrovsky @ 2016-03-22 20:54 UTC (permalink / raw)
  To: Firas Azar, Andrew Cooper, xen-devel

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


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

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

end of thread, other threads:[~2016-03-22 20:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-22 20:06 [PATCH] pygrub: remove isconfig option Firas Azar
2016-03-22 20:22 ` Andrew Cooper
2016-03-22 20:46   ` Firas Azar
2016-03-22 20:54     ` Boris Ostrovsky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).