| Message ID | 1432644847-7566-1-git-send-email-michael.i.doherty@intel.com |
|---|---|
| State | New |
| Headers | show |
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 >
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.
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 > >
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).
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 >>> >
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.
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
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.
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 > >
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 */ };
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/sysconfigs/target/target-x86_64.conf b/sysconfigs/target/target-x86_64.conf deleted file mode 100644 index e69de29..0000000