Bug 795322

Summary: add_ro should return error if not running in a config state
Product: Red Hat Enterprise Linux 6 Reporter: Mohua Li <moli>
Component: libguestfsAssignee: Richard W.M. Jones <rjones>
Status: CLOSED ERRATA QA Contact: Virtualization Bugs <virt-bugs>
Severity: medium Docs Contact:
Priority: medium    
Version: 6.3CC: gaowanlong, leiwang, qguan, qwan
Target Milestone: rcKeywords: Regression
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: libguestfs-1.16.6-1.el6 Doc Type: Bug Fix
Doc Text:
No Documentation needed.
Story Points: ---
Clone Of:
: 796523 (view as bug list) Environment:
Last Closed: 2012-06-20 07:00:40 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Mohua Li 2012-02-20 08:59:58 UTC
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:

Comment 1 Qixiang Wan 2012-02-20 09:45:43 UTC
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.

Comment 2 Richard W.M. Jones 2012-02-20 11:10:00 UTC
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.

Comment 3 Richard W.M. Jones 2012-02-20 11:10:49 UTC
(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.

Comment 5 Wanlong Gao 2012-02-23 05:25:43 UTC
(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

Comment 6 Wanlong Gao 2012-02-23 05:53:53 UTC
(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

Comment 7 Richard W.M. Jones 2012-03-01 16:40:51 UTC
*** Bug 796523 has been marked as a duplicate of this bug. ***

Comment 10 Richard W.M. Jones 2012-04-26 13:44:57 UTC
    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.

Comment 12 errata-xmlrpc 2012-06-20 07:00:40 UTC
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