Hide Forgot
Description of problem: run add_ro in a ready state(after launch), didn't return an error, which might quite confused, as it didn't really added the disk to the applicance, you could get the info from qemu-kvm cmdline(file=/tmp/test1.img etc), so better to retrun an error if add_ro not running in a config state Version-Release number of selected component (if applicable): libguestfs-1.16.5-1.el6.x86_64 How reproducible: always Steps to Reproduce: 1.guestfish -xv -N fs 2.add-ro rhevh.img 3.list-filesystems Actual results: add-ro not running in a config state, didn't return an error Expected results: add-ro not running in a config state, return an error Additional info:
There are many other commands working like this, so it sounds reasonable to find a good solution to make it work for other commands too. And there is another reason to get this fixed, the commands like 'add-ro' won't take effect after launch, but the original parameters can be lost if we want to check that. for example: # guestfish -a /dev/null [...] ><fs> run ><fs> get-smp 1 ><fs> set-smp 4 ><fs> get-smp 4 ><fs> Actually, we're expecting to get 1 rather than 4, because there is only one vcpu assigned to the appliance, and it's impossible for the appliance to get 4 virtual vcpus now.
I see what's happened here is a subtle regression in this commit: https://github.com/libguestfs/libguestfs/commit/f1041e912b72116d66274d2f15e50ce34a9531fd The old path used to call guestfs__add_drive_opts -> guestfs__config -> add_cmdline, and add_cmdline contains this check: if (g->state != CONFIG) { error (g, _("command line cannot be altered after qemu subprocess launched")); return -1; } However after that commit, guestfs__add_drive_opts adds the drive to an internal list of drives and doesn't immediately call add_cmdline. Thus the error check above is never performed. Therefore this bug is a regression from RHEL 6.2.
(In reply to comment #1) > There are many other commands working like this, so it sounds reasonable to > find a good solution to make it work for other commands too. > > And there is another reason to get this fixed, the commands like 'add-ro' won't > take effect after launch, but the original parameters can be lost if we want to > check that. for example: > > # guestfish -a /dev/null > [...] > ><fs> run > ><fs> get-smp > 1 > ><fs> set-smp 4 > ><fs> get-smp > 4 > ><fs> > > Actually, we're expecting to get 1 rather than 4, because there is only one > vcpu assigned to the appliance, and it's impossible for the appliance to get 4 > virtual vcpus now. Just to note this is a separate issue and requires a new BZ.
(In reply to comment #1) > There are many other commands working like this, so it sounds reasonable to > find a good solution to make it work for other commands too. > > And there is another reason to get this fixed, the commands like 'add-ro' won't > take effect after launch, but the original parameters can be lost if we want to > check that. for example: > > # guestfish -a /dev/null > [...] > ><fs> run > ><fs> get-smp > 1 > ><fs> set-smp 4 > ><fs> get-smp > 4 > ><fs> > > Actually, we're expecting to get 1 rather than 4, because there is only one > vcpu assigned to the appliance, and it's impossible for the appliance to get 4 > virtual vcpus now. A fix patch has sent against upstream. https://www.redhat.com/archives/libguestfs/2012-February/msg00082.html
(In reply to comment #3) > (In reply to comment #1) > > There are many other commands working like this, so it sounds reasonable to > > find a good solution to make it work for other commands too. > > > > And there is another reason to get this fixed, the commands like 'add-ro' won't > > take effect after launch, but the original parameters can be lost if we want to > > check that. for example: > > > > # guestfish -a /dev/null > > [...] > > ><fs> run > > ><fs> get-smp > > 1 > > ><fs> set-smp 4 > > ><fs> get-smp > > 4 > > ><fs> > > > > Actually, we're expecting to get 1 rather than 4, because there is only one > > vcpu assigned to the appliance, and it's impossible for the appliance to get 4 > > virtual vcpus now. > > Just to note this is a separate issue and requires a new BZ. New BZ created to https://bugzilla.redhat.com/show_bug.cgi?id=796523
*** Bug 796523 has been marked as a duplicate of this bug. ***
Technical note added. If any revisions are required, please edit the "Technical Notes" field accordingly. All revisions will be proofread by the Engineering Content Services team. New Contents: No Documentation needed.
Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. http://rhn.redhat.com/errata/RHSA-2012-0774.html