Bug 796520

Summary:	[RFE] Prevent user from running some appliance configure commands after appliance boot up
Product:	Red Hat Enterprise Linux 6	Reporter:	Qixiang Wan <qwan>
Component:	libguestfs	Assignee:	Richard W.M. Jones <rjones>
Status:	CLOSED ERRATA	QA Contact:	Virtualization Bugs <virt-bugs>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	6.2	CC:	gaowanlong, leiwang, moli, qguan, yuzhou
Target Milestone:	rc	Keywords:	FutureFeature
Target Release:	6.3
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:	libguestfs-1.16.8-1.el6	Doc Type:	Enhancement
Doc Text:	No Documentation needed	Story Points:	---
Clone Of:		Environment:
Last Closed:	2012-06-20 07:00:43 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 Qixiang Wan 2012-02-23 05:42:25 UTC

Description of problem:
There are some commands which are used for configuring the appliance, and they won't take effect after appliance boot up. But user can still run these commands after the appliance has been launched, beside of this, if user want to check the configurations which the appliance was boot up with, the value could be wrong if the configure commands were executed with new values after appliance boot up.

There is an example in bug 795322 comment 1.

Here is a list of the commands (but not the full list):
set-append
set-attach-method
set-memsize
set-qemu
set-path
set-smp

If we don't support hot-plug block/cdrom devices after appliance boot up, then some of the add-drive-* commands should also been considered.

Version-Release number of selected component (if applicable):
libguestfs-1.16.5-1.el6

How reproducible:
100%

Steps to Reproduce:

Bug 795322 comment 1
  
Actual results:
User can run the commands won't take effect after appliance boot up.

Expected results:
Only allow users to run these commands when it's in 'config' state.

Additional info:

Comment 1 Richard W.M. Jones 2012-02-23 08:37:57 UTC

Wanlong Gao posted a partial patch to fix this here:

https://www.redhat.com/archives/libguestfs/2012-February/msg00082.html

Actually that patch is wrong because it doesn't set
error along the error path and return -1.

However I think there should be a common way to fix both
this bug and bug 795322:

In the generator, it should be possible to mark non_daemon_functions
that can only be called in the CONFIG state.  Or mark
non_daemon_functions that can not be called in the CONFIG
state (whichever is less change).  Then the generator can
create the appropriate wrapper code to test the state.

Comment 2 Richard W.M. Jones 2012-02-23 08:44:26 UTC

non_daemon_functions that should only be called in
CONFIG state:

add_cdrom
add_domain
add_drive
add_drive_opts
add_drive_ro
add_drive_ro_with_if
add_drive_with_if
add_libvirt_dom
config
launch
set_append
set_attach_method
set_autosync
set_direct
set_memsize
set_network
set_path
set_pgroup
set_qemu
set_recovery_proc
set_selinux
set_smp
set_trace
set_verbose
wait_ready

non_daemon_functions that can be called in any state:

debug_cmdline
debug_drives
file_architecture
get_append
get_attach_method
get_autosync
get_direct
get_memsize
get_network
get_path
get_pgroup
get_pid
get_qemu
get_recovery_proc
get_selinux
get_smp
get_state
get_trace
get_verbose
inspect_get_arch
inspect_get_distro
inspect_get_drive_mappings
inspect_get_filesystems
inspect_get_format
inspect_get_hostname
inspect_get_icon
inspect_get_major_version
inspect_get_minor_version
inspect_get_mountpoints
inspect_get_package_format
inspect_get_package_management
inspect_get_product_name
inspect_get_product_variant
inspect_get_roots
inspect_get_type
inspect_get_windows_current_control_set
inspect_get_windows_systemroot
inspect_is_live
inspect_is_multipart
inspect_is_netinst
inspect_list_applications
inspect_os
is_busy
is_config
is_launching
is_ready
kill_subprocess
list_filesystems

So the list is about even.  I suggest adding a ConfigOnly
flag to the flags.

Comment 3 Qixiang Wan 2012-02-23 09:06:03 UTC

(In reply to comment #2)
> non_daemon_functions that should only be called in
> CONFIG state:
> ...
> set_trace
> set_verbose
> ...

set_trace, set_verbose can work well after appliance boot up, so we don't need to make it only work in CONFIG state. And user may want to use them before/after special commands for debugging rather than a global configuration.

Comment 4 Wanlong Gao 2012-02-25 01:06:07 UTC

Rich has pushed a upstream patch.
https://github.com/libguestfs/libguestfs/commit/9e5c0b39c6b598a733a790d73d27eae491910a22

Comment 7 Richard W.M. Jones 2012-03-05 08:52:22 UTC

guestfs_set_autosync affects the guestfs_close call and
does not need to be called only in the config state
(see bug 799798).

Setting back to ASSIGNED.

Comment 8 Richard W.M. Jones 2012-03-05 08:59:27 UTC

We need to include upstream commit
715f7e28090d08c16bfdd32b0298812972c12b36.

Comment 10 Qixiang Wan 2012-03-15 03:27:39 UTC

Verified with libguestfs-1.16.10-1.el6.

The following APIs are changed to be "config only":

launch
add_drive
add_cdrom
add_drive_ro
config
set_qemu
set_path
set_append
set_memsize
set_selinux
set_direct
set_recovery_proc
add_drive_with_if
add_drive_ro_with_if
set_network
add_drive_opts
add_domain
set_attach_method
set_pgroup
set_smp

Comment 11 Richard W.M. Jones 2012-04-26 12:20:40 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 13 errata-xmlrpc 2012-06-20 07:00:43 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