From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53527) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XWnW3-0005HV-NY for qemu-devel@nongnu.org; Wed, 24 Sep 2014 10:26:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XWnVx-00069K-Hp for qemu-devel@nongnu.org; Wed, 24 Sep 2014 10:25:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:9218) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XWnVx-00068d-Ac for qemu-devel@nongnu.org; Wed, 24 Sep 2014 10:25:53 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s8OEPlAu024723 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Wed, 24 Sep 2014 10:25:47 -0400 From: Markus Armbruster References: <1411490885-29782-1-git-send-email-jsnow@redhat.com> <1411490885-29782-4-git-send-email-jsnow@redhat.com> Date: Wed, 24 Sep 2014 16:25:45 +0200 In-Reply-To: <1411490885-29782-4-git-send-email-jsnow@redhat.com> (John Snow's message of "Tue, 23 Sep 2014 12:48:02 -0400") Message-ID: <87mw9pp02e.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 3/6] pc/vl: Add units-per-default-bus property List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow Cc: kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, mst@redhat.com John Snow writes: > This patch adds the 'units_per_default_bus' property > which allows individual boards to declare their desired > index => (bus,unit) mapping for their default HBA, so > that boards such as Q35 can specify that its default > if_ide HBA, AHCI, only accepts one unit per bus. > > This property only overrides the mapping for drives > matching the block_default_type interface. > > Signed-off-by: John Snow A bit more detail would be nice here. Suggest to give a -drive example, and discuss its meaning before and after the patch with Q35: bogus unit# before, sane one after. > --- > hw/i386/pc.c | 1 + > hw/i386/pc_q35.c | 3 ++- > include/hw/boards.h | 2 ++ > vl.c | 8 ++++++++ > 4 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 2c2e9dc..ab578bf 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1524,6 +1524,7 @@ static void pc_generic_machine_class_init(ObjectClass *oc, void *data) > mc->hot_add_cpu = qm->hot_add_cpu; > mc->kvm_type = qm->kvm_type; > mc->block_default_type = qm->block_default_type; > + mc->units_per_default_bus = qm->units_per_default_bus; > mc->max_cpus = qm->max_cpus; > mc->no_serial = qm->no_serial; > mc->no_parallel = qm->no_parallel; > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index d4a907c..b28ddbb 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -344,7 +344,8 @@ static void pc_q35_init_1_4(MachineState *machine) > #define PC_Q35_MACHINE_OPTIONS \ > PC_DEFAULT_MACHINE_OPTIONS, \ > .desc = "Standard PC (Q35 + ICH9, 2009)", \ > - .hot_add_cpu = pc_hot_add_cpu > + .hot_add_cpu = pc_hot_add_cpu, \ > + .units_per_default_bus = 1 Changes the (bus, unit) mapping for all Q35 machine types. Overriding the overide by adding .units_per_default_bus = 0 to PC_Q35_2_1_MACHINE_OPTIONS would limit the change to new machine types, if we want that for bug compatibility. I'm not sure it would be useful, I'm just spelling it out for the benefit of other reviewers. Any incompatible change to old machine types must feature *prominently* in the commit message, even when it changes things from "not useful" to "actually sane". > #define PC_Q35_2_2_MACHINE_OPTIONS \ > PC_Q35_MACHINE_OPTIONS, \ > diff --git a/include/hw/boards.h b/include/hw/boards.h > index dfb6718..663f16a 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -28,6 +28,7 @@ struct QEMUMachine { > QEMUMachineHotAddCPUFunc *hot_add_cpu; > QEMUMachineGetKvmtypeFunc *kvm_type; > BlockInterfaceType block_default_type; > + int units_per_default_bus; > int max_cpus; > unsigned int no_serial:1, > no_parallel:1, > @@ -86,6 +87,7 @@ struct MachineClass { > int (*kvm_type)(const char *arg); > > BlockInterfaceType block_default_type; > + int units_per_default_bus; > int max_cpus; > unsigned int no_serial:1, > no_parallel:1, > diff --git a/vl.c b/vl.c > index eaef240..6fffa1f 100644 > --- a/vl.c > +++ b/vl.c > @@ -1588,6 +1588,7 @@ static void machine_class_init(ObjectClass *oc, void *data) > mc->hot_add_cpu = qm->hot_add_cpu; > mc->kvm_type = qm->kvm_type; > mc->block_default_type = qm->block_default_type; > + mc->units_per_default_bus = qm->units_per_default_bus; > mc->max_cpus = qm->max_cpus; > mc->no_serial = qm->no_serial; > mc->no_parallel = qm->no_parallel; > @@ -4377,6 +4378,13 @@ int main(int argc, char **argv, char **envp) > blk_mig_init(); > ram_mig_init(); > > + /* If the currently selected machine wishes to override the units-per-bus > + * property of its default HBA interface type, do so now. */ > + if (machine_class->units_per_default_bus) { > + override_max_devs(machine_class->block_default_type, > + machine_class->units_per_default_bus); > + } > + > /* open the virtual block devices */ > if (snapshot) > qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot, NULL, 0); Looks good.