* [Qemu-devel] [PATCH] arch_init: Use stateless configuration for default target_${target}.conf
@ 2015-05-22 15:42 Ikey Doherty
2015-05-22 21:58 ` Eric Blake
0 siblings, 1 reply; 22+ messages in thread
From: Ikey Doherty @ 2015-05-22 15:42 UTC (permalink / raw)
To: qemu-devel
The goal of stateless, and thus this change, is to separate OS configuration
from system administrator configuration. With this change we will read the
default configuration data from /usr/share/defaults/qemu, in the absence of
an overriding site administrator configuration in /etc/qemu.
A key advantage of this change is enabling a sane and immutable default OS
configuration, that is resiliant to upgrades. Ultimate power is still left
to the system administrator, with the ability to override the defaults if
required. Lastly, given that the sane defaults are always available, the
administrator may simply remove their site-config files to reset the
configuration to the "factory defaults" (i.e. OS configuration).
Signed-off-by: Ikey Doherty <michael.i.doherty@intel.com>
---
Makefile | 8 ++++----
arch_init.c | 1 +
configure | 2 ++
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/Makefile b/Makefile
index d945804..52635da 100644
--- a/Makefile
+++ b/Makefile
@@ -390,12 +390,12 @@ endif
endif
install-confdir:
- $(INSTALL_DIR) "$(DESTDIR)$(qemu_confdir)"
+ $(INSTALL_DIR) "$(DESTDIR)$(qemu_defaultdir)"
-install-sysconfig: install-datadir install-confdir
- $(INSTALL_DATA) $(SRC_PATH)/sysconfigs/target/target-x86_64.conf "$(DESTDIR)$(qemu_confdir)"
+install-defaultconfig: install-datadir install-confdir
+ $(INSTALL_DATA) $(SRC_PATH)/sysconfigs/target/target-x86_64.conf "$(DESTDIR)$(qemu_defaultdir)"
-install: all $(if $(BUILD_DOCS),install-doc) install-sysconfig \
+install: all $(if $(BUILD_DOCS),install-doc) install-defaultconfig \
install-datadir install-localstatedir
ifneq ($(TOOLS),)
$(call install-prog,$(TOOLS),$(DESTDIR)$(bindir))
diff --git a/arch_init.c b/arch_init.c
index 23d3feb..f23fb1f 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -137,6 +137,7 @@ static struct defconfig_file {
} default_config_files[] = {
{ CONFIG_QEMU_CONFDIR "/qemu.conf", true },
{ CONFIG_QEMU_CONFDIR "/target-" TARGET_NAME ".conf", true },
+ { CONFIG_QEMU_DEFAULTDIR "/target-" TARGET_NAME ".conf", true },
{ NULL }, /* end of list */
};
diff --git a/configure b/configure
index f758f32..a0b6477 100755
--- a/configure
+++ b/configure
@@ -4303,6 +4303,7 @@ fi
qemu_confdir=$sysconfdir$confsuffix
qemu_moddir=$libdir$confsuffix
qemu_datadir=$datadir$confsuffix
+qemu_defaultdir="$datadir/defaults$confsuffix"
qemu_localedir="$datadir/locale"
tools=""
@@ -4543,6 +4544,7 @@ echo "mandir=$mandir" >> $config_host_mak
echo "sysconfdir=$sysconfdir" >> $config_host_mak
echo "qemu_confdir=$qemu_confdir" >> $config_host_mak
echo "qemu_datadir=$qemu_datadir" >> $config_host_mak
+echo "qemu_defaultdir=$qemu_defaultdir" >> $config_host_mak
echo "qemu_docdir=$qemu_docdir" >> $config_host_mak
echo "qemu_moddir=$qemu_moddir" >> $config_host_mak
if test "$mingw32" = "no" ; then
--
1.9.1
---------------------------------------------------------------------
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] arch_init: Use stateless configuration for default target_${target}.conf
2015-05-22 15:42 [Qemu-devel] [PATCH] arch_init: Use stateless configuration for default target_${target}.conf Ikey Doherty
@ 2015-05-22 21:58 ` Eric Blake
2015-05-26 11:11 ` [Qemu-devel] [PATCH v2] " Ikey Doherty
2015-05-26 11:13 ` [Qemu-devel] [PATCH] arch_init: Use stateless configuration for default target_${target}.conf Ikey Doherty
0 siblings, 2 replies; 22+ messages in thread
From: Eric Blake @ 2015-05-22 21:58 UTC (permalink / raw)
To: Ikey Doherty, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 925 bytes --]
On 05/22/2015 09:42 AM, Ikey Doherty wrote:
meta-comment:
> 1.9.1
>
> ---------------------------------------------------------------------
> Intel Corporation (UK) Limited
> Registered No. 1134945 (England)
> Registered Office: Pipers Way, Swindon SN3 1RJ
> VAT No: 860 2173 47
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
This disclaimer is unenforceable on publicly-archived mailing lists.
Can you tweak your configuration so that mails from you don't spam the
list with legalese, since some readers are uncomfortable answering
emails with a bad disclaimer?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v2] arch_init: Use stateless configuration for default target_${target}.conf
2015-05-22 21:58 ` Eric Blake
@ 2015-05-26 11:11 ` Ikey Doherty
2015-05-26 11:23 ` Paolo Bonzini
2015-05-26 11:13 ` [Qemu-devel] [PATCH] arch_init: Use stateless configuration for default target_${target}.conf Ikey Doherty
1 sibling, 1 reply; 22+ messages in thread
From: Ikey Doherty @ 2015-05-26 11:11 UTC (permalink / raw)
To: qemu-devel
The goal of stateless, and thus this change, is to separate OS configuration
from system administrator configuration. With this change we will read the
default configuration data from /usr/share/defaults/qemu, in the absence of
an overriding site administrator configuration in /etc/qemu.
A key advantage of this change is enabling a sane and immutable default OS
configuration, that is resiliant to upgrades. Ultimate power is still left
to the system administrator, with the ability to override the defaults if
required. Lastly, given that the sane defaults are always available, the
administrator may simply remove their site-config files to reset the
configuration to the "factory defaults" (i.e. OS configuration).
Signed-off-by: Ikey Doherty <michael.i.doherty@intel.com>
---
Makefile | 8 ++++----
arch_init.c | 1 +
configure | 2 ++
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/Makefile b/Makefile
index d945804..52635da 100644
--- a/Makefile
+++ b/Makefile
@@ -390,12 +390,12 @@ endif
endif
install-confdir:
- $(INSTALL_DIR) "$(DESTDIR)$(qemu_confdir)"
+ $(INSTALL_DIR) "$(DESTDIR)$(qemu_defaultdir)"
-install-sysconfig: install-datadir install-confdir
- $(INSTALL_DATA) $(SRC_PATH)/sysconfigs/target/target-x86_64.conf "$(DESTDIR)$(qemu_confdir)"
+install-defaultconfig: install-datadir install-confdir
+ $(INSTALL_DATA) $(SRC_PATH)/sysconfigs/target/target-x86_64.conf "$(DESTDIR)$(qemu_defaultdir)"
-install: all $(if $(BUILD_DOCS),install-doc) install-sysconfig \
+install: all $(if $(BUILD_DOCS),install-doc) install-defaultconfig \
install-datadir install-localstatedir
ifneq ($(TOOLS),)
$(call install-prog,$(TOOLS),$(DESTDIR)$(bindir))
diff --git a/arch_init.c b/arch_init.c
index 23d3feb..f23fb1f 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -137,6 +137,7 @@ static struct defconfig_file {
} default_config_files[] = {
{ CONFIG_QEMU_CONFDIR "/qemu.conf", true },
{ CONFIG_QEMU_CONFDIR "/target-" TARGET_NAME ".conf", true },
+ { CONFIG_QEMU_DEFAULTDIR "/target-" TARGET_NAME ".conf", true },
{ NULL }, /* end of list */
};
diff --git a/configure b/configure
index f758f32..a0b6477 100755
--- a/configure
+++ b/configure
@@ -4303,6 +4303,7 @@ fi
qemu_confdir=$sysconfdir$confsuffix
qemu_moddir=$libdir$confsuffix
qemu_datadir=$datadir$confsuffix
+qemu_defaultdir="$datadir/defaults$confsuffix"
qemu_localedir="$datadir/locale"
tools=""
@@ -4543,6 +4544,7 @@ echo "mandir=$mandir" >> $config_host_mak
echo "sysconfdir=$sysconfdir" >> $config_host_mak
echo "qemu_confdir=$qemu_confdir" >> $config_host_mak
echo "qemu_datadir=$qemu_datadir" >> $config_host_mak
+echo "qemu_defaultdir=$qemu_defaultdir" >> $config_host_mak
echo "qemu_docdir=$qemu_docdir" >> $config_host_mak
echo "qemu_moddir=$qemu_moddir" >> $config_host_mak
if test "$mingw32" = "no" ; then
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v2] arch_init: Use stateless configuration for default target_${target}.conf
2015-05-26 11:11 ` [Qemu-devel] [PATCH v2] " Ikey Doherty
@ 2015-05-26 11:23 ` Paolo Bonzini
2015-05-26 12:54 ` [Qemu-devel] [PATCH v3 1/2] arch_init: Drop target-x86_64.conf Ikey Doherty
0 siblings, 1 reply; 22+ messages in thread
From: Paolo Bonzini @ 2015-05-26 11:23 UTC (permalink / raw)
To: Ikey Doherty, qemu-devel
On 26/05/2015 13:11, Ikey Doherty wrote:
> The goal of stateless, and thus this change, is to separate OS configuration
> from system administrator configuration. With this change we will read the
> default configuration data from /usr/share/defaults/qemu, in the absence of
> an overriding site administrator configuration in /etc/qemu.
>
> A key advantage of this change is enabling a sane and immutable default OS
> configuration, that is resiliant to upgrades. Ultimate power is still left
> to the system administrator, with the ability to override the defaults if
> required. Lastly, given that the sane defaults are always available, the
> administrator may simply remove their site-config files to reset the
> configuration to the "factory defaults" (i.e. OS configuration).
The patch does make sense, however the target-x86_64.conf file has now
been empty for three years and we probably should just stop honoring it.
In addition, there is another file in /etc/qemu, namely bridge.conf.
Stateless configuration would make a lot of sense for that file. Can
you modify your patch to kill target-$TARGET_NAME.conf altogether, and
to use stateless configuration for bridge.conf?
Paolo
> Signed-off-by: Ikey Doherty <michael.i.doherty@intel.com>
> ---
> Makefile | 8 ++++----
> arch_init.c | 1 +
> configure | 2 ++
> 3 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index d945804..52635da 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -390,12 +390,12 @@ endif
> endif
>
> install-confdir:
> - $(INSTALL_DIR) "$(DESTDIR)$(qemu_confdir)"
> + $(INSTALL_DIR) "$(DESTDIR)$(qemu_defaultdir)"
>
> -install-sysconfig: install-datadir install-confdir
> - $(INSTALL_DATA) $(SRC_PATH)/sysconfigs/target/target-x86_64.conf "$(DESTDIR)$(qemu_confdir)"
> +install-defaultconfig: install-datadir install-confdir
> + $(INSTALL_DATA) $(SRC_PATH)/sysconfigs/target/target-x86_64.conf "$(DESTDIR)$(qemu_defaultdir)"
>
> -install: all $(if $(BUILD_DOCS),install-doc) install-sysconfig \
> +install: all $(if $(BUILD_DOCS),install-doc) install-defaultconfig \
> install-datadir install-localstatedir
> ifneq ($(TOOLS),)
> $(call install-prog,$(TOOLS),$(DESTDIR)$(bindir))
> diff --git a/arch_init.c b/arch_init.c
> index 23d3feb..f23fb1f 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -137,6 +137,7 @@ static struct defconfig_file {
> } default_config_files[] = {
> { CONFIG_QEMU_CONFDIR "/qemu.conf", true },
> { CONFIG_QEMU_CONFDIR "/target-" TARGET_NAME ".conf", true },
> + { CONFIG_QEMU_DEFAULTDIR "/target-" TARGET_NAME ".conf", true },
> { NULL }, /* end of list */
> };
>
> diff --git a/configure b/configure
> index f758f32..a0b6477 100755
> --- a/configure
> +++ b/configure
> @@ -4303,6 +4303,7 @@ fi
> qemu_confdir=$sysconfdir$confsuffix
> qemu_moddir=$libdir$confsuffix
> qemu_datadir=$datadir$confsuffix
> +qemu_defaultdir="$datadir/defaults$confsuffix"
> qemu_localedir="$datadir/locale"
>
> tools=""
> @@ -4543,6 +4544,7 @@ echo "mandir=$mandir" >> $config_host_mak
> echo "sysconfdir=$sysconfdir" >> $config_host_mak
> echo "qemu_confdir=$qemu_confdir" >> $config_host_mak
> echo "qemu_datadir=$qemu_datadir" >> $config_host_mak
> +echo "qemu_defaultdir=$qemu_defaultdir" >> $config_host_mak
> echo "qemu_docdir=$qemu_docdir" >> $config_host_mak
> echo "qemu_moddir=$qemu_moddir" >> $config_host_mak
> if test "$mingw32" = "no" ; then
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v3 1/2] arch_init: Drop target-x86_64.conf
2015-05-26 11:23 ` Paolo Bonzini
@ 2015-05-26 12:54 ` Ikey Doherty
2015-05-26 12:54 ` [Qemu-devel] [PATCH v3 2/2] qemu-bridge-helper: Use stateless configuration for bridge.conf Ikey Doherty
` (3 more replies)
0 siblings, 4 replies; 22+ messages in thread
From: Ikey Doherty @ 2015-05-26 12:54 UTC (permalink / raw)
To: qemu-devel
The target-x86_64.conf sysconfig file has been empty and essentially ignored
now for several years. This change removes the unused file to enable moving
towards a stateless configuration.
Signed-off-by: Ikey Doherty <michael.i.doherty@intel.com>
---
Makefile | 7 +------
arch_init.c | 1 -
sysconfigs/target/target-x86_64.conf | 0
3 files changed, 1 insertion(+), 7 deletions(-)
delete mode 100644 sysconfigs/target/target-x86_64.conf
diff --git a/Makefile b/Makefile
index d945804..2d52536 100644
--- a/Makefile
+++ b/Makefile
@@ -389,13 +389,8 @@ ifneq (,$(findstring qemu-ga,$(TOOLS)))
endif
endif
-install-confdir:
- $(INSTALL_DIR) "$(DESTDIR)$(qemu_confdir)"
-install-sysconfig: install-datadir install-confdir
- $(INSTALL_DATA) $(SRC_PATH)/sysconfigs/target/target-x86_64.conf "$(DESTDIR)$(qemu_confdir)"
-
-install: all $(if $(BUILD_DOCS),install-doc) install-sysconfig \
+install: all $(if $(BUILD_DOCS),install-doc) \
install-datadir install-localstatedir
ifneq ($(TOOLS),)
$(call install-prog,$(TOOLS),$(DESTDIR)$(bindir))
diff --git a/arch_init.c b/arch_init.c
index 23d3feb..b5d90a4 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -136,7 +136,6 @@ static struct defconfig_file {
bool userconfig;
} default_config_files[] = {
{ CONFIG_QEMU_CONFDIR "/qemu.conf", true },
- { CONFIG_QEMU_CONFDIR "/target-" TARGET_NAME ".conf", true },
{ NULL }, /* end of list */
};
diff --git a/sysconfigs/target/target-x86_64.conf b/sysconfigs/target/target-x86_64.conf
deleted file mode 100644
index e69de29..0000000
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v3 2/2] qemu-bridge-helper: Use stateless configuration for bridge.conf
2015-05-26 12:54 ` [Qemu-devel] [PATCH v3 1/2] arch_init: Drop target-x86_64.conf Ikey Doherty
@ 2015-05-26 12:54 ` Ikey Doherty
2015-05-26 14:00 ` Paolo Bonzini
` (2 more replies)
2015-05-26 14:00 ` [Qemu-devel] [PATCH v3 1/2] arch_init: Drop target-x86_64.conf Paolo Bonzini
` (2 subsequent siblings)
3 siblings, 3 replies; 22+ messages in thread
From: Ikey Doherty @ 2015-05-26 12:54 UTC (permalink / raw)
To: qemu-devel
The goal of stateless, and thus this change, is to separate OS configuration
from system administrator configuration. With this change we will read the
default configuration data from /usr/share/defaults/qemu, in the absence of
an overriding site administrator configuration in /etc/qemu.
A key advantage of this change is enabling a sane and immutable default OS
configuration, that is resiliant to upgrades. Ultimate power is still left
to the system administrator, with the ability to override the defaults if
required. Lastly, given that the sane defaults are always available, the
administrator may simply remove their site-config files to reset the
configuration to the "factory defaults" (i.e. OS configuration).
Signed-off-by: Ikey Doherty <michael.i.doherty@intel.com>
---
configure | 2 ++
qemu-bridge-helper.c | 15 +++++++++------
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/configure b/configure
index f758f32..a0b6477 100755
--- a/configure
+++ b/configure
@@ -4303,6 +4303,7 @@ fi
qemu_confdir=$sysconfdir$confsuffix
qemu_moddir=$libdir$confsuffix
qemu_datadir=$datadir$confsuffix
+qemu_defaultdir="$datadir/defaults$confsuffix"
qemu_localedir="$datadir/locale"
tools=""
@@ -4543,6 +4544,7 @@ echo "mandir=$mandir" >> $config_host_mak
echo "sysconfdir=$sysconfdir" >> $config_host_mak
echo "qemu_confdir=$qemu_confdir" >> $config_host_mak
echo "qemu_datadir=$qemu_datadir" >> $config_host_mak
+echo "qemu_defaultdir=$qemu_defaultdir" >> $config_host_mak
echo "qemu_docdir=$qemu_docdir" >> $config_host_mak
echo "qemu_moddir=$qemu_moddir" >> $config_host_mak
if test "$mingw32" = "no" ; then
diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
index 36eb3bc..0f795f4 100644
--- a/qemu-bridge-helper.c
+++ b/qemu-bridge-helper.c
@@ -47,7 +47,8 @@
#include <cap-ng.h>
#endif
-#define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
+#define DEFAULT_ACL_FILE CONFIG_QEMU_DEFAULTDIR "/bridge.conf"
+#define SITE_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
enum {
ACL_ALLOW = 0,
@@ -272,11 +273,13 @@ int main(int argc, char **argv)
/* parse default acl file */
QSIMPLEQ_INIT(&acl_list);
- if (parse_acl_file(DEFAULT_ACL_FILE, &acl_list) == -1) {
- fprintf(stderr, "failed to parse default acl file `%s'\n",
- DEFAULT_ACL_FILE);
- ret = EXIT_FAILURE;
- goto cleanup;
+ if (parse_acl_file(SITE_ACL_FILE, &acl_list) == -1) {
+ if (parse_acl_file(DEFAULT_ACL_FILE, &acl_list) == -1) {
+ fprintf(stderr, "failed to parse default acl file `%s'\n",
+ DEFAULT_ACL_FILE);
+ ret = EXIT_FAILURE;
+ goto cleanup;
+ }
}
/* validate bridge against acl -- default policy is to deny
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] qemu-bridge-helper: Use stateless configuration for bridge.conf
2015-05-26 12:54 ` [Qemu-devel] [PATCH v3 2/2] qemu-bridge-helper: Use stateless configuration for bridge.conf Ikey Doherty
@ 2015-05-26 14:00 ` Paolo Bonzini
2015-05-26 16:38 ` Eduardo Habkost
2015-05-27 14:00 ` Stefan Hajnoczi
2 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2015-05-26 14:00 UTC (permalink / raw)
To: Ikey Doherty, qemu-devel, Stefan Hajnoczi
CCing maintainer.
Paolo
On 26/05/2015 14:54, Ikey Doherty wrote:
> The goal of stateless, and thus this change, is to separate OS configuration
> from system administrator configuration. With this change we will read the
> default configuration data from /usr/share/defaults/qemu, in the absence of
> an overriding site administrator configuration in /etc/qemu.
>
> A key advantage of this change is enabling a sane and immutable default OS
> configuration, that is resiliant to upgrades. Ultimate power is still left
> to the system administrator, with the ability to override the defaults if
> required. Lastly, given that the sane defaults are always available, the
> administrator may simply remove their site-config files to reset the
> configuration to the "factory defaults" (i.e. OS configuration).
>
> Signed-off-by: Ikey Doherty <michael.i.doherty@intel.com>
> ---
> configure | 2 ++
> qemu-bridge-helper.c | 15 +++++++++------
> 2 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/configure b/configure
> index f758f32..a0b6477 100755
> --- a/configure
> +++ b/configure
> @@ -4303,6 +4303,7 @@ fi
> qemu_confdir=$sysconfdir$confsuffix
> qemu_moddir=$libdir$confsuffix
> qemu_datadir=$datadir$confsuffix
> +qemu_defaultdir="$datadir/defaults$confsuffix"
> qemu_localedir="$datadir/locale"
>
> tools=""
> @@ -4543,6 +4544,7 @@ echo "mandir=$mandir" >> $config_host_mak
> echo "sysconfdir=$sysconfdir" >> $config_host_mak
> echo "qemu_confdir=$qemu_confdir" >> $config_host_mak
> echo "qemu_datadir=$qemu_datadir" >> $config_host_mak
> +echo "qemu_defaultdir=$qemu_defaultdir" >> $config_host_mak
> echo "qemu_docdir=$qemu_docdir" >> $config_host_mak
> echo "qemu_moddir=$qemu_moddir" >> $config_host_mak
> if test "$mingw32" = "no" ; then
> diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
> index 36eb3bc..0f795f4 100644
> --- a/qemu-bridge-helper.c
> +++ b/qemu-bridge-helper.c
> @@ -47,7 +47,8 @@
> #include <cap-ng.h>
> #endif
>
> -#define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
> +#define DEFAULT_ACL_FILE CONFIG_QEMU_DEFAULTDIR "/bridge.conf"
> +#define SITE_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
>
> enum {
> ACL_ALLOW = 0,
> @@ -272,11 +273,13 @@ int main(int argc, char **argv)
>
> /* parse default acl file */
> QSIMPLEQ_INIT(&acl_list);
> - if (parse_acl_file(DEFAULT_ACL_FILE, &acl_list) == -1) {
> - fprintf(stderr, "failed to parse default acl file `%s'\n",
> - DEFAULT_ACL_FILE);
> - ret = EXIT_FAILURE;
> - goto cleanup;
> + if (parse_acl_file(SITE_ACL_FILE, &acl_list) == -1) {
> + if (parse_acl_file(DEFAULT_ACL_FILE, &acl_list) == -1) {
> + fprintf(stderr, "failed to parse default acl file `%s'\n",
> + DEFAULT_ACL_FILE);
> + ret = EXIT_FAILURE;
> + goto cleanup;
> + }
> }
>
> /* validate bridge against acl -- default policy is to deny
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] qemu-bridge-helper: Use stateless configuration for bridge.conf
2015-05-26 12:54 ` [Qemu-devel] [PATCH v3 2/2] qemu-bridge-helper: Use stateless configuration for bridge.conf Ikey Doherty
2015-05-26 14:00 ` Paolo Bonzini
@ 2015-05-26 16:38 ` Eduardo Habkost
2015-05-26 16:41 ` Ikey Doherty
2015-05-27 14:00 ` Stefan Hajnoczi
2 siblings, 1 reply; 22+ messages in thread
From: Eduardo Habkost @ 2015-05-26 16:38 UTC (permalink / raw)
To: Ikey Doherty; +Cc: qemu-devel, Stefan Hajnoczi
On Tue, May 26, 2015 at 01:54:07PM +0100, Ikey Doherty wrote:
> The goal of stateless, and thus this change, is to separate OS configuration
> from system administrator configuration. With this change we will read the
> default configuration data from /usr/share/defaults/qemu, in the absence of
> an overriding site administrator configuration in /etc/qemu.
>
> A key advantage of this change is enabling a sane and immutable default OS
> configuration, that is resiliant to upgrades. Ultimate power is still left
> to the system administrator, with the ability to override the defaults if
> required. Lastly, given that the sane defaults are always available, the
> administrator may simply remove their site-config files to reset the
> configuration to the "factory defaults" (i.e. OS configuration).
>
> Signed-off-by: Ikey Doherty <michael.i.doherty@intel.com>
> ---
> configure | 2 ++
> qemu-bridge-helper.c | 15 +++++++++------
> 2 files changed, 11 insertions(+), 6 deletions(-)
>
[...]
> -#define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
> +#define DEFAULT_ACL_FILE CONFIG_QEMU_DEFAULTDIR "/bridge.conf"
> +#define SITE_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
>
> enum {
> ACL_ALLOW = 0,
> @@ -272,11 +273,13 @@ int main(int argc, char **argv)
>
> /* parse default acl file */
> QSIMPLEQ_INIT(&acl_list);
> - if (parse_acl_file(DEFAULT_ACL_FILE, &acl_list) == -1) {
> - fprintf(stderr, "failed to parse default acl file `%s'\n",
> - DEFAULT_ACL_FILE);
> - ret = EXIT_FAILURE;
> - goto cleanup;
> + if (parse_acl_file(SITE_ACL_FILE, &acl_list) == -1) {
> + if (parse_acl_file(DEFAULT_ACL_FILE, &acl_list) == -1) {
> + fprintf(stderr, "failed to parse default acl file `%s'\n",
> + DEFAULT_ACL_FILE);
> + ret = EXIT_FAILURE;
> + goto cleanup;
> + }
> }
This will make syntax errors on SITE_ACL_FILE cause partial loading of
the rules on SITE_ACL_FILE, and trigger loading of DEFAULT_ACL_FILE,
instead of aborting bridge-helper.
Wouldn't it be better to fallback to DEFAULT_ACL_FILE if and only if
SITE_ACL_FILE is missing?
--
Eduardo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] qemu-bridge-helper: Use stateless configuration for bridge.conf
2015-05-26 16:38 ` Eduardo Habkost
@ 2015-05-26 16:41 ` Ikey Doherty
2015-05-26 16:57 ` Eduardo Habkost
0 siblings, 1 reply; 22+ messages in thread
From: Ikey Doherty @ 2015-05-26 16:41 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: qemu-devel, Stefan Hajnoczi
On 26/05/15 17:38, Eduardo Habkost wrote:
> On Tue, May 26, 2015 at 01:54:07PM +0100, Ikey Doherty wrote:
>> The goal of stateless, and thus this change, is to separate OS configuration
>> from system administrator configuration. With this change we will read the
>> default configuration data from /usr/share/defaults/qemu, in the absence of
>> an overriding site administrator configuration in /etc/qemu.
>>
>> A key advantage of this change is enabling a sane and immutable default OS
>> configuration, that is resiliant to upgrades. Ultimate power is still left
>> to the system administrator, with the ability to override the defaults if
>> required. Lastly, given that the sane defaults are always available, the
>> administrator may simply remove their site-config files to reset the
>> configuration to the "factory defaults" (i.e. OS configuration).
>>
>> Signed-off-by: Ikey Doherty <michael.i.doherty@intel.com>
>> ---
>> configure | 2 ++
>> qemu-bridge-helper.c | 15 +++++++++------
>> 2 files changed, 11 insertions(+), 6 deletions(-)
>>
> [...]
>> -#define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
>> +#define DEFAULT_ACL_FILE CONFIG_QEMU_DEFAULTDIR "/bridge.conf"
>> +#define SITE_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
>>
>> enum {
>> ACL_ALLOW = 0,
>> @@ -272,11 +273,13 @@ int main(int argc, char **argv)
>>
>> /* parse default acl file */
>> QSIMPLEQ_INIT(&acl_list);
>> - if (parse_acl_file(DEFAULT_ACL_FILE, &acl_list) == -1) {
>> - fprintf(stderr, "failed to parse default acl file `%s'\n",
>> - DEFAULT_ACL_FILE);
>> - ret = EXIT_FAILURE;
>> - goto cleanup;
>> + if (parse_acl_file(SITE_ACL_FILE, &acl_list) == -1) {
>> + if (parse_acl_file(DEFAULT_ACL_FILE, &acl_list) == -1) {
>> + fprintf(stderr, "failed to parse default acl file `%s'\n",
>> + DEFAULT_ACL_FILE);
>> + ret = EXIT_FAILURE;
>> + goto cleanup;
>> + }
>> }
>
> This will make syntax errors on SITE_ACL_FILE cause partial loading of
> the rules on SITE_ACL_FILE, and trigger loading of DEFAULT_ACL_FILE,
> instead of aborting bridge-helper.
>
> Wouldn't it be better to fallback to DEFAULT_ACL_FILE if and only if
> SITE_ACL_FILE is missing?
>
I could stat the file and fallback to default, and only parse_acl_file
the one that exists. Better?
--
Clear Linux Project for Intel Architecture
Intel Open Source Technology Center
http://www.clearlinux.org
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] qemu-bridge-helper: Use stateless configuration for bridge.conf
2015-05-26 16:41 ` Ikey Doherty
@ 2015-05-26 16:57 ` Eduardo Habkost
2015-05-27 14:02 ` Stefan Hajnoczi
0 siblings, 1 reply; 22+ messages in thread
From: Eduardo Habkost @ 2015-05-26 16:57 UTC (permalink / raw)
To: Ikey Doherty; +Cc: qemu-devel, Stefan Hajnoczi
On Tue, May 26, 2015 at 05:41:08PM +0100, Ikey Doherty wrote:
> On 26/05/15 17:38, Eduardo Habkost wrote:
> >On Tue, May 26, 2015 at 01:54:07PM +0100, Ikey Doherty wrote:
> >>The goal of stateless, and thus this change, is to separate OS configuration
> >>from system administrator configuration. With this change we will read the
> >>default configuration data from /usr/share/defaults/qemu, in the absence of
> >>an overriding site administrator configuration in /etc/qemu.
> >>
> >>A key advantage of this change is enabling a sane and immutable default OS
> >>configuration, that is resiliant to upgrades. Ultimate power is still left
> >>to the system administrator, with the ability to override the defaults if
> >>required. Lastly, given that the sane defaults are always available, the
> >>administrator may simply remove their site-config files to reset the
> >>configuration to the "factory defaults" (i.e. OS configuration).
> >>
> >>Signed-off-by: Ikey Doherty <michael.i.doherty@intel.com>
> >>---
> >> configure | 2 ++
> >> qemu-bridge-helper.c | 15 +++++++++------
> >> 2 files changed, 11 insertions(+), 6 deletions(-)
> >>
> >[...]
> >>-#define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
> >>+#define DEFAULT_ACL_FILE CONFIG_QEMU_DEFAULTDIR "/bridge.conf"
> >>+#define SITE_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
> >>
> >> enum {
> >> ACL_ALLOW = 0,
> >>@@ -272,11 +273,13 @@ int main(int argc, char **argv)
> >>
> >> /* parse default acl file */
> >> QSIMPLEQ_INIT(&acl_list);
> >>- if (parse_acl_file(DEFAULT_ACL_FILE, &acl_list) == -1) {
> >>- fprintf(stderr, "failed to parse default acl file `%s'\n",
> >>- DEFAULT_ACL_FILE);
> >>- ret = EXIT_FAILURE;
> >>- goto cleanup;
> >>+ if (parse_acl_file(SITE_ACL_FILE, &acl_list) == -1) {
> >>+ if (parse_acl_file(DEFAULT_ACL_FILE, &acl_list) == -1) {
> >>+ fprintf(stderr, "failed to parse default acl file `%s'\n",
> >>+ DEFAULT_ACL_FILE);
> >>+ ret = EXIT_FAILURE;
> >>+ goto cleanup;
> >>+ }
> >> }
> >
> >This will make syntax errors on SITE_ACL_FILE cause partial loading of
> >the rules on SITE_ACL_FILE, and trigger loading of DEFAULT_ACL_FILE,
> >instead of aborting bridge-helper.
> >
> >Wouldn't it be better to fallback to DEFAULT_ACL_FILE if and only if
> >SITE_ACL_FILE is missing?
> >
>
> I could stat the file and fallback to default, and only parse_acl_file
> the one that exists. Better?
Or you could simply call parse_acl_file(DEFAULT_ACL_FILE) only if
parse_acl_file(SITE_ACL_FILE) set errno=ENOENT.
--
Eduardo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] qemu-bridge-helper: Use stateless configuration for bridge.conf
2015-05-26 16:57 ` Eduardo Habkost
@ 2015-05-27 14:02 ` Stefan Hajnoczi
0 siblings, 0 replies; 22+ messages in thread
From: Stefan Hajnoczi @ 2015-05-27 14:02 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: Ikey Doherty, Stefan Hajnoczi, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2920 bytes --]
On Tue, May 26, 2015 at 01:57:41PM -0300, Eduardo Habkost wrote:
> On Tue, May 26, 2015 at 05:41:08PM +0100, Ikey Doherty wrote:
> > On 26/05/15 17:38, Eduardo Habkost wrote:
> > >On Tue, May 26, 2015 at 01:54:07PM +0100, Ikey Doherty wrote:
> > >>The goal of stateless, and thus this change, is to separate OS configuration
> > >>from system administrator configuration. With this change we will read the
> > >>default configuration data from /usr/share/defaults/qemu, in the absence of
> > >>an overriding site administrator configuration in /etc/qemu.
> > >>
> > >>A key advantage of this change is enabling a sane and immutable default OS
> > >>configuration, that is resiliant to upgrades. Ultimate power is still left
> > >>to the system administrator, with the ability to override the defaults if
> > >>required. Lastly, given that the sane defaults are always available, the
> > >>administrator may simply remove their site-config files to reset the
> > >>configuration to the "factory defaults" (i.e. OS configuration).
> > >>
> > >>Signed-off-by: Ikey Doherty <michael.i.doherty@intel.com>
> > >>---
> > >> configure | 2 ++
> > >> qemu-bridge-helper.c | 15 +++++++++------
> > >> 2 files changed, 11 insertions(+), 6 deletions(-)
> > >>
> > >[...]
> > >>-#define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
> > >>+#define DEFAULT_ACL_FILE CONFIG_QEMU_DEFAULTDIR "/bridge.conf"
> > >>+#define SITE_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
> > >>
> > >> enum {
> > >> ACL_ALLOW = 0,
> > >>@@ -272,11 +273,13 @@ int main(int argc, char **argv)
> > >>
> > >> /* parse default acl file */
> > >> QSIMPLEQ_INIT(&acl_list);
> > >>- if (parse_acl_file(DEFAULT_ACL_FILE, &acl_list) == -1) {
> > >>- fprintf(stderr, "failed to parse default acl file `%s'\n",
> > >>- DEFAULT_ACL_FILE);
> > >>- ret = EXIT_FAILURE;
> > >>- goto cleanup;
> > >>+ if (parse_acl_file(SITE_ACL_FILE, &acl_list) == -1) {
> > >>+ if (parse_acl_file(DEFAULT_ACL_FILE, &acl_list) == -1) {
> > >>+ fprintf(stderr, "failed to parse default acl file `%s'\n",
> > >>+ DEFAULT_ACL_FILE);
> > >>+ ret = EXIT_FAILURE;
> > >>+ goto cleanup;
> > >>+ }
> > >> }
> > >
> > >This will make syntax errors on SITE_ACL_FILE cause partial loading of
> > >the rules on SITE_ACL_FILE, and trigger loading of DEFAULT_ACL_FILE,
> > >instead of aborting bridge-helper.
> > >
> > >Wouldn't it be better to fallback to DEFAULT_ACL_FILE if and only if
> > >SITE_ACL_FILE is missing?
> > >
> >
> > I could stat the file and fallback to default, and only parse_acl_file
> > the one that exists. Better?
>
> Or you could simply call parse_acl_file(DEFAULT_ACL_FILE) only if
> parse_acl_file(SITE_ACL_FILE) set errno=ENOENT.
Yes, please.
Stefan
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] qemu-bridge-helper: Use stateless configuration for bridge.conf
2015-05-26 12:54 ` [Qemu-devel] [PATCH v3 2/2] qemu-bridge-helper: Use stateless configuration for bridge.conf Ikey Doherty
2015-05-26 14:00 ` Paolo Bonzini
2015-05-26 16:38 ` Eduardo Habkost
@ 2015-05-27 14:00 ` Stefan Hajnoczi
2 siblings, 0 replies; 22+ messages in thread
From: Stefan Hajnoczi @ 2015-05-27 14:00 UTC (permalink / raw)
To: Ikey Doherty; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2090 bytes --]
On Tue, May 26, 2015 at 01:54:07PM +0100, Ikey Doherty wrote:
> The goal of stateless, and thus this change, is to separate OS configuration
> from system administrator configuration. With this change we will read the
> default configuration data from /usr/share/defaults/qemu, in the absence of
> an overriding site administrator configuration in /etc/qemu.
>
> A key advantage of this change is enabling a sane and immutable default OS
> configuration, that is resiliant to upgrades. Ultimate power is still left
> to the system administrator, with the ability to override the defaults if
> required. Lastly, given that the sane defaults are always available, the
> administrator may simply remove their site-config files to reset the
> configuration to the "factory defaults" (i.e. OS configuration).
>
> Signed-off-by: Ikey Doherty <michael.i.doherty@intel.com>
> ---
> configure | 2 ++
> qemu-bridge-helper.c | 15 +++++++++------
> 2 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/configure b/configure
> index f758f32..a0b6477 100755
> --- a/configure
> +++ b/configure
> @@ -4303,6 +4303,7 @@ fi
> qemu_confdir=$sysconfdir$confsuffix
> qemu_moddir=$libdir$confsuffix
> qemu_datadir=$datadir$confsuffix
> +qemu_defaultdir="$datadir/defaults$confsuffix"
> qemu_localedir="$datadir/locale"
>
> tools=""
> @@ -4543,6 +4544,7 @@ echo "mandir=$mandir" >> $config_host_mak
> echo "sysconfdir=$sysconfdir" >> $config_host_mak
> echo "qemu_confdir=$qemu_confdir" >> $config_host_mak
> echo "qemu_datadir=$qemu_datadir" >> $config_host_mak
> +echo "qemu_defaultdir=$qemu_defaultdir" >> $config_host_mak
> echo "qemu_docdir=$qemu_docdir" >> $config_host_mak
> echo "qemu_moddir=$qemu_moddir" >> $config_host_mak
> if test "$mingw32" = "no" ; then
Changes to ./configure would make a nice separate commit where you can
explain the file layout for stateless configuration.
That way it's also easier to cherry-pick or revert the ./configure
change without bringing in the qemu-bridge-helper.c change.
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] arch_init: Drop target-x86_64.conf
2015-05-26 12:54 ` [Qemu-devel] [PATCH v3 1/2] arch_init: Drop target-x86_64.conf Ikey Doherty
2015-05-26 12:54 ` [Qemu-devel] [PATCH v3 2/2] qemu-bridge-helper: Use stateless configuration for bridge.conf Ikey Doherty
@ 2015-05-26 14:00 ` Paolo Bonzini
2015-05-26 16:25 ` Eduardo Habkost
2015-05-26 15:37 ` Eric Blake
2015-05-26 17:01 ` Eduardo Habkost
3 siblings, 1 reply; 22+ messages in thread
From: Paolo Bonzini @ 2015-05-26 14:00 UTC (permalink / raw)
To: Ikey Doherty, qemu-devel, Eduardo Habkost
CCing maintainer.
Paolo
On 26/05/2015 14:54, Ikey Doherty wrote:
> The target-x86_64.conf sysconfig file has been empty and essentially ignored
> now for several years. This change removes the unused file to enable moving
> towards a stateless configuration.
>
> Signed-off-by: Ikey Doherty <michael.i.doherty@intel.com>
> ---
> Makefile | 7 +------
> arch_init.c | 1 -
> sysconfigs/target/target-x86_64.conf | 0
> 3 files changed, 1 insertion(+), 7 deletions(-)
> delete mode 100644 sysconfigs/target/target-x86_64.conf
>
> diff --git a/Makefile b/Makefile
> index d945804..2d52536 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -389,13 +389,8 @@ ifneq (,$(findstring qemu-ga,$(TOOLS)))
> endif
> endif
>
> -install-confdir:
> - $(INSTALL_DIR) "$(DESTDIR)$(qemu_confdir)"
>
> -install-sysconfig: install-datadir install-confdir
> - $(INSTALL_DATA) $(SRC_PATH)/sysconfigs/target/target-x86_64.conf "$(DESTDIR)$(qemu_confdir)"
> -
> -install: all $(if $(BUILD_DOCS),install-doc) install-sysconfig \
> +install: all $(if $(BUILD_DOCS),install-doc) \
> install-datadir install-localstatedir
> ifneq ($(TOOLS),)
> $(call install-prog,$(TOOLS),$(DESTDIR)$(bindir))
> diff --git a/arch_init.c b/arch_init.c
> index 23d3feb..b5d90a4 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -136,7 +136,6 @@ static struct defconfig_file {
> bool userconfig;
> } default_config_files[] = {
> { CONFIG_QEMU_CONFDIR "/qemu.conf", true },
> - { CONFIG_QEMU_CONFDIR "/target-" TARGET_NAME ".conf", true },
> { NULL }, /* end of list */
> };
>
> diff --git a/sysconfigs/target/target-x86_64.conf b/sysconfigs/target/target-x86_64.conf
> deleted file mode 100644
> index e69de29..0000000
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] arch_init: Drop target-x86_64.conf
2015-05-26 14:00 ` [Qemu-devel] [PATCH v3 1/2] arch_init: Drop target-x86_64.conf Paolo Bonzini
@ 2015-05-26 16:25 ` Eduardo Habkost
2015-05-26 16:29 ` Paolo Bonzini
2015-05-26 16:30 ` Ikey Doherty
0 siblings, 2 replies; 22+ messages in thread
From: Eduardo Habkost @ 2015-05-26 16:25 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Ikey Doherty, qemu-devel
On Tue, May 26, 2015 at 04:00:45PM +0200, Paolo Bonzini wrote:
> CCing maintainer.
>
> Paolo
>
> On 26/05/2015 14:54, Ikey Doherty wrote:
> > The target-x86_64.conf sysconfig file has been empty and essentially ignored
> > now for several years. This change removes the unused file to enable moving
> > towards a stateless configuration.
> >
> > Signed-off-by: Ikey Doherty <michael.i.doherty@intel.com>
Can you separate this into two patches? First deleting the empty
target-x86_64.conf file from the tree & Makefile, then another patch
deleting the
{ CONFIG_QEMU_CONFDIR "/target-" TARGET_NAME ".conf", true }
line in arch_init.c?
We can delete sysconfigs/target/target-x86_64.conf from our source tree
immediately, but I am not sure we should disable loading of
/etc/qemu/target-*.conf with no warning (users may have their own
target-*.conf files in their systems).
We should probably warn about it in the 2.4 release announcement, and
remove the arch_init.c line in 2.5.
I would even go further and argue for removing /etc/qemu config file
auto-loading entirely in QEMU 2.5 (including qemu.conf and
target-*.conf).
> > ---
> > Makefile | 7 +------
> > arch_init.c | 1 -
> > sysconfigs/target/target-x86_64.conf | 0
> > 3 files changed, 1 insertion(+), 7 deletions(-)
> > delete mode 100644 sysconfigs/target/target-x86_64.conf
> >
> > diff --git a/Makefile b/Makefile
> > index d945804..2d52536 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -389,13 +389,8 @@ ifneq (,$(findstring qemu-ga,$(TOOLS)))
> > endif
> > endif
> >
> > -install-confdir:
> > - $(INSTALL_DIR) "$(DESTDIR)$(qemu_confdir)"
> >
> > -install-sysconfig: install-datadir install-confdir
> > - $(INSTALL_DATA) $(SRC_PATH)/sysconfigs/target/target-x86_64.conf "$(DESTDIR)$(qemu_confdir)"
> > -
> > -install: all $(if $(BUILD_DOCS),install-doc) install-sysconfig \
> > +install: all $(if $(BUILD_DOCS),install-doc) \
> > install-datadir install-localstatedir
> > ifneq ($(TOOLS),)
> > $(call install-prog,$(TOOLS),$(DESTDIR)$(bindir))
> > diff --git a/arch_init.c b/arch_init.c
> > index 23d3feb..b5d90a4 100644
> > --- a/arch_init.c
> > +++ b/arch_init.c
> > @@ -136,7 +136,6 @@ static struct defconfig_file {
> > bool userconfig;
> > } default_config_files[] = {
> > { CONFIG_QEMU_CONFDIR "/qemu.conf", true },
> > - { CONFIG_QEMU_CONFDIR "/target-" TARGET_NAME ".conf", true },
> > { NULL }, /* end of list */
> > };
> >
> > diff --git a/sysconfigs/target/target-x86_64.conf b/sysconfigs/target/target-x86_64.conf
> > deleted file mode 100644
> > index e69de29..0000000
> >
--
Eduardo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] arch_init: Drop target-x86_64.conf
2015-05-26 16:25 ` Eduardo Habkost
@ 2015-05-26 16:29 ` Paolo Bonzini
2015-05-26 16:40 ` Eduardo Habkost
2015-05-26 16:30 ` Ikey Doherty
1 sibling, 1 reply; 22+ messages in thread
From: Paolo Bonzini @ 2015-05-26 16:29 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: Ikey Doherty, qemu-devel
On 26/05/2015 18:25, Eduardo Habkost wrote:
> Can you separate this into two patches? First deleting the empty
> target-x86_64.conf file from the tree & Makefile, then another patch
> deleting the
> { CONFIG_QEMU_CONFDIR "/target-" TARGET_NAME ".conf", true }
> line in arch_init.c?
>
> We can delete sysconfigs/target/target-x86_64.conf from our source tree
> immediately, but I am not sure we should disable loading of
> /etc/qemu/target-*.conf with no warning (users may have their own
> target-*.conf files in their systems).
What is the usecase? Was /etc/qemu/target-*.conf actually meant to be
user-customizable when it hosted the CPU models?
Paolo
> We should probably warn about it in the 2.4 release announcement, and
> remove the arch_init.c line in 2.5.
>
> I would even go further and argue for removing /etc/qemu config file
> auto-loading entirely in QEMU 2.5 (including qemu.conf and
> target-*.conf).
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] arch_init: Drop target-x86_64.conf
2015-05-26 16:29 ` Paolo Bonzini
@ 2015-05-26 16:40 ` Eduardo Habkost
2015-05-26 16:51 ` Paolo Bonzini
0 siblings, 1 reply; 22+ messages in thread
From: Eduardo Habkost @ 2015-05-26 16:40 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Ikey Doherty, qemu-devel
On Tue, May 26, 2015 at 06:29:25PM +0200, Paolo Bonzini wrote:
>
>
> On 26/05/2015 18:25, Eduardo Habkost wrote:
> > Can you separate this into two patches? First deleting the empty
> > target-x86_64.conf file from the tree & Makefile, then another patch
> > deleting the
> > { CONFIG_QEMU_CONFDIR "/target-" TARGET_NAME ".conf", true }
> > line in arch_init.c?
> >
> > We can delete sysconfigs/target/target-x86_64.conf from our source tree
> > immediately, but I am not sure we should disable loading of
> > /etc/qemu/target-*.conf with no warning (users may have their own
> > target-*.conf files in their systems).
>
> What is the usecase? Was /etc/qemu/target-*.conf actually meant to be
> user-customizable when it hosted the CPU models?
I don't know what's the use case, that's why I think we should really
remove it. I only want to warn users before removing it, because I don't
know if people are using it for anything else.
--
Eduardo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] arch_init: Drop target-x86_64.conf
2015-05-26 16:40 ` Eduardo Habkost
@ 2015-05-26 16:51 ` Paolo Bonzini
2015-05-26 16:59 ` Eduardo Habkost
0 siblings, 1 reply; 22+ messages in thread
From: Paolo Bonzini @ 2015-05-26 16:51 UTC (permalink / raw)
To: Eduardo Habkost; +Cc: Ikey Doherty, qemu-devel
On 26/05/2015 18:40, Eduardo Habkost wrote:
> > What is the usecase? Was /etc/qemu/target-*.conf actually meant to be
> > user-customizable when it hosted the CPU models?
>
> I don't know what's the use case, that's why I think we should really
> remove it. I only want to warn users before removing it, because I don't
> know if people are using it for anything else.
There are definitely usecases for qemu.conf (iqn/username/password
tuples for libiscsi, globally disabling KSM or memory dumps, globally
enabling timestamps). Not that anyone's using it, but I would leave it
alone.
However, I don't think target-x86_64.conf would be missed, and it's
traditionally been something that is "owned by the package" (because the
CPU models were there) so we should be able to treat it as private.
Paolo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] arch_init: Drop target-x86_64.conf
2015-05-26 16:51 ` Paolo Bonzini
@ 2015-05-26 16:59 ` Eduardo Habkost
0 siblings, 0 replies; 22+ messages in thread
From: Eduardo Habkost @ 2015-05-26 16:59 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Ikey Doherty, qemu-devel
On Tue, May 26, 2015 at 06:51:18PM +0200, Paolo Bonzini wrote:
>
>
> On 26/05/2015 18:40, Eduardo Habkost wrote:
> > > What is the usecase? Was /etc/qemu/target-*.conf actually meant to be
> > > user-customizable when it hosted the CPU models?
> >
> > I don't know what's the use case, that's why I think we should really
> > remove it. I only want to warn users before removing it, because I don't
> > know if people are using it for anything else.
>
> There are definitely usecases for qemu.conf (iqn/username/password
> tuples for libiscsi, globally disabling KSM or memory dumps, globally
> enabling timestamps). Not that anyone's using it, but I would leave it
> alone.
>
> However, I don't think target-x86_64.conf would be missed, and it's
> traditionally been something that is "owned by the package" (because the
> CPU models were there) so we should be able to treat it as private.
You convinced me. We can apply this patch and if we change our minds
before 2.4.0 we can easily revert just the one-line arch_init.c. So
there's no need to split this patch in two, anymore.
--
Eduardo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] arch_init: Drop target-x86_64.conf
2015-05-26 16:25 ` Eduardo Habkost
2015-05-26 16:29 ` Paolo Bonzini
@ 2015-05-26 16:30 ` Ikey Doherty
1 sibling, 0 replies; 22+ messages in thread
From: Ikey Doherty @ 2015-05-26 16:30 UTC (permalink / raw)
To: Eduardo Habkost, Paolo Bonzini; +Cc: qemu-devel
On 26/05/15 17:25, Eduardo Habkost wrote:
> On Tue, May 26, 2015 at 04:00:45PM +0200, Paolo Bonzini wrote:
>> CCing maintainer.
>>
>> Paolo
>>
>> On 26/05/2015 14:54, Ikey Doherty wrote:
>>> The target-x86_64.conf sysconfig file has been empty and essentially ignored
>>> now for several years. This change removes the unused file to enable moving
>>> towards a stateless configuration.
>>>
>>> Signed-off-by: Ikey Doherty <michael.i.doherty@intel.com>
>
> Can you separate this into two patches? First deleting the empty
> target-x86_64.conf file from the tree & Makefile, then another patch
> deleting the
> { CONFIG_QEMU_CONFDIR "/target-" TARGET_NAME ".conf", true }
> line in arch_init.c?
>
Ack. End of day here, will submit a fresh patch-set tomorrow drawing
any further conversations had here into consideration.
> We can delete sysconfigs/target/target-x86_64.conf from our source tree
> immediately, but I am not sure we should disable loading of
> /etc/qemu/target-*.conf with no warning (users may have their own
> target-*.conf files in their systems).
Sure.
>
> We should probably warn about it in the 2.4 release announcement, and
> remove the arch_init.c line in 2.5.
>
> I would even go further and argue for removing /etc/qemu config file
> auto-loading entirely in QEMU 2.5 (including qemu.conf and
> target-*.conf).
If that's needed/agreed-upon by tomorrow I can add that too, as another
patch.
-ikey
>
>>> ---
>>> Makefile | 7 +------
>>> arch_init.c | 1 -
>>> sysconfigs/target/target-x86_64.conf | 0
>>> 3 files changed, 1 insertion(+), 7 deletions(-)
>>> delete mode 100644 sysconfigs/target/target-x86_64.conf
>>>
>>> diff --git a/Makefile b/Makefile
>>> index d945804..2d52536 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -389,13 +389,8 @@ ifneq (,$(findstring qemu-ga,$(TOOLS)))
>>> endif
>>> endif
>>>
>>> -install-confdir:
>>> - $(INSTALL_DIR) "$(DESTDIR)$(qemu_confdir)"
>>>
>>> -install-sysconfig: install-datadir install-confdir
>>> - $(INSTALL_DATA) $(SRC_PATH)/sysconfigs/target/target-x86_64.conf "$(DESTDIR)$(qemu_confdir)"
>>> -
>>> -install: all $(if $(BUILD_DOCS),install-doc) install-sysconfig \
>>> +install: all $(if $(BUILD_DOCS),install-doc) \
>>> install-datadir install-localstatedir
>>> ifneq ($(TOOLS),)
>>> $(call install-prog,$(TOOLS),$(DESTDIR)$(bindir))
>>> diff --git a/arch_init.c b/arch_init.c
>>> index 23d3feb..b5d90a4 100644
>>> --- a/arch_init.c
>>> +++ b/arch_init.c
>>> @@ -136,7 +136,6 @@ static struct defconfig_file {
>>> bool userconfig;
>>> } default_config_files[] = {
>>> { CONFIG_QEMU_CONFDIR "/qemu.conf", true },
>>> - { CONFIG_QEMU_CONFDIR "/target-" TARGET_NAME ".conf", true },
>>> { NULL }, /* end of list */
>>> };
>>>
>>> diff --git a/sysconfigs/target/target-x86_64.conf b/sysconfigs/target/target-x86_64.conf
>>> deleted file mode 100644
>>> index e69de29..0000000
>>>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] arch_init: Drop target-x86_64.conf
2015-05-26 12:54 ` [Qemu-devel] [PATCH v3 1/2] arch_init: Drop target-x86_64.conf Ikey Doherty
2015-05-26 12:54 ` [Qemu-devel] [PATCH v3 2/2] qemu-bridge-helper: Use stateless configuration for bridge.conf Ikey Doherty
2015-05-26 14:00 ` [Qemu-devel] [PATCH v3 1/2] arch_init: Drop target-x86_64.conf Paolo Bonzini
@ 2015-05-26 15:37 ` Eric Blake
2015-05-26 17:01 ` Eduardo Habkost
3 siblings, 0 replies; 22+ messages in thread
From: Eric Blake @ 2015-05-26 15:37 UTC (permalink / raw)
To: Ikey Doherty, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1055 bytes --]
On 05/26/2015 06:54 AM, Ikey Doherty wrote:
[meta-comment]
> The target-x86_64.conf sysconfig file has been empty and essentially ignored
> now for several years. This change removes the unused file to enable moving
> towards a stateless configuration.
>
> Signed-off-by: Ikey Doherty <michael.i.doherty@intel.com>
> ---
> Makefile | 7 +------
> arch_init.c | 1 -
> sysconfigs/target/target-x86_64.conf | 0
> 3 files changed, 1 insertion(+), 7 deletions(-)
> delete mode 100644 sysconfigs/target/target-x86_64.conf
When sending a v3 patch, it's better to do it as a new top-level thread
rather than in-reply-to an existing thread (several maintainers use
scripting tools that only look for top-level threads, and skip patches
that are buried deep within threads). Also, when sending more than one
patch in a series, include a 0/2 cover letter for the patches.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] arch_init: Drop target-x86_64.conf
2015-05-26 12:54 ` [Qemu-devel] [PATCH v3 1/2] arch_init: Drop target-x86_64.conf Ikey Doherty
` (2 preceding siblings ...)
2015-05-26 15:37 ` Eric Blake
@ 2015-05-26 17:01 ` Eduardo Habkost
3 siblings, 0 replies; 22+ messages in thread
From: Eduardo Habkost @ 2015-05-26 17:01 UTC (permalink / raw)
To: Ikey Doherty; +Cc: qemu-devel
On Tue, May 26, 2015 at 01:54:06PM +0100, Ikey Doherty wrote:
> The target-x86_64.conf sysconfig file has been empty and essentially ignored
> now for several years. This change removes the unused file to enable moving
> towards a stateless configuration.
>
> Signed-off-by: Ikey Doherty <michael.i.doherty@intel.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
I am queueing it in the x86 tree. Thanks!
> ---
> Makefile | 7 +------
> arch_init.c | 1 -
> sysconfigs/target/target-x86_64.conf | 0
> 3 files changed, 1 insertion(+), 7 deletions(-)
> delete mode 100644 sysconfigs/target/target-x86_64.conf
>
> diff --git a/Makefile b/Makefile
> index d945804..2d52536 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -389,13 +389,8 @@ ifneq (,$(findstring qemu-ga,$(TOOLS)))
> endif
> endif
>
> -install-confdir:
> - $(INSTALL_DIR) "$(DESTDIR)$(qemu_confdir)"
>
> -install-sysconfig: install-datadir install-confdir
> - $(INSTALL_DATA) $(SRC_PATH)/sysconfigs/target/target-x86_64.conf "$(DESTDIR)$(qemu_confdir)"
> -
> -install: all $(if $(BUILD_DOCS),install-doc) install-sysconfig \
> +install: all $(if $(BUILD_DOCS),install-doc) \
> install-datadir install-localstatedir
> ifneq ($(TOOLS),)
> $(call install-prog,$(TOOLS),$(DESTDIR)$(bindir))
> diff --git a/arch_init.c b/arch_init.c
> index 23d3feb..b5d90a4 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -136,7 +136,6 @@ static struct defconfig_file {
> bool userconfig;
> } default_config_files[] = {
> { CONFIG_QEMU_CONFDIR "/qemu.conf", true },
> - { CONFIG_QEMU_CONFDIR "/target-" TARGET_NAME ".conf", true },
> { NULL }, /* end of list */
> };
>
> diff --git a/sysconfigs/target/target-x86_64.conf b/sysconfigs/target/target-x86_64.conf
> deleted file mode 100644
> index e69de29..0000000
> --
> 1.9.1
>
>
--
Eduardo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] arch_init: Use stateless configuration for default target_${target}.conf
2015-05-22 21:58 ` Eric Blake
2015-05-26 11:11 ` [Qemu-devel] [PATCH v2] " Ikey Doherty
@ 2015-05-26 11:13 ` Ikey Doherty
1 sibling, 0 replies; 22+ messages in thread
From: Ikey Doherty @ 2015-05-26 11:13 UTC (permalink / raw)
To: qemu-devel
On 22/05/15 22:58, Eric Blake wrote:
> On 05/22/2015 09:42 AM, Ikey Doherty wrote:
>
> meta-comment:
>
>> 1.9.1
>>
>> ---------------------------------------------------------------------
>> Intel Corporation (UK) Limited
>> Registered No. 1134945 (England)
>> Registered Office: Pipers Way, Swindon SN3 1RJ
>> VAT No: 860 2173 47
>>
>> This e-mail and any attachments may contain confidential material for
>> the sole use of the intended recipient(s). Any review or distribution
>> by others is strictly prohibited. If you are not the intended
>> recipient, please contact the sender and delete all copies.
>
> This disclaimer is unenforceable on publicly-archived mailing lists.
> Can you tweak your configuration so that mails from you don't spam the
> list with legalese, since some readers are uncomfortable answering
> emails with a bad disclaimer?
>
My apologies, I thought I'd resolved that prior to sending. That's now
corrected and I've resent.
- ikey
--
Clear Linux Project for Intel Architecture
Intel Open Source Technology Center
http://www.clearlinux.org
---------------------------------------------------------------------
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2015-05-27 14:02 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-22 15:42 [Qemu-devel] [PATCH] arch_init: Use stateless configuration for default target_${target}.conf Ikey Doherty
2015-05-22 21:58 ` Eric Blake
2015-05-26 11:11 ` [Qemu-devel] [PATCH v2] " Ikey Doherty
2015-05-26 11:23 ` Paolo Bonzini
2015-05-26 12:54 ` [Qemu-devel] [PATCH v3 1/2] arch_init: Drop target-x86_64.conf Ikey Doherty
2015-05-26 12:54 ` [Qemu-devel] [PATCH v3 2/2] qemu-bridge-helper: Use stateless configuration for bridge.conf Ikey Doherty
2015-05-26 14:00 ` Paolo Bonzini
2015-05-26 16:38 ` Eduardo Habkost
2015-05-26 16:41 ` Ikey Doherty
2015-05-26 16:57 ` Eduardo Habkost
2015-05-27 14:02 ` Stefan Hajnoczi
2015-05-27 14:00 ` Stefan Hajnoczi
2015-05-26 14:00 ` [Qemu-devel] [PATCH v3 1/2] arch_init: Drop target-x86_64.conf Paolo Bonzini
2015-05-26 16:25 ` Eduardo Habkost
2015-05-26 16:29 ` Paolo Bonzini
2015-05-26 16:40 ` Eduardo Habkost
2015-05-26 16:51 ` Paolo Bonzini
2015-05-26 16:59 ` Eduardo Habkost
2015-05-26 16:30 ` Ikey Doherty
2015-05-26 15:37 ` Eric Blake
2015-05-26 17:01 ` Eduardo Habkost
2015-05-26 11:13 ` [Qemu-devel] [PATCH] arch_init: Use stateless configuration for default target_${target}.conf Ikey Doherty
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).