Re: [Qemu-devel] [PATCH v2 4/7] bootindex: delete bootindex when device is removed
Hi, +del_boot_device_path(dev); You can call this from device_finalize() instead of placing it into each individual device. Maybe put this call in device_finalize is not a good idea. I have three reasons: 1. the device's some memory have been freed before call device_finalize, such as device-id. It is too later. I don't think you even need id. See my reply to v4 2/8. But you have a point about being too late: some devices call add_boot_device_path() on realize, so those would need to revert the operation on unrealize; others do it on init, so they need to do it on finalize. On either case, I believe an extra check inside device_finalize() wouldn't hurt, even if it becomes redundant on some devices. OK. And I copy your review from v3 2/7, as follows: What if the device doesn't have any ID set? I don't see anything on add_boot_device_path() ensuring that dev-id is always set. Yes, the id is not always set. So, I add a check in V4. Why you don't just check if i-dev == dev? No, if we check directly i-dev == dev, we will not handle the virtio devices. For example, the common user configure a virtio-net nic using command line like -device virtio-net-pci,netdev=net0,bootindex=3,id=nic1 . Then the id property will be added for virtio-net-pci device, not virtio-net device which stored in the global fw_boot_order list. So, the i-dev When common users want to change the bootindex of virtio-net. They only are concerned that they have configured an id for virtio-net nic card. So, they can pass the id to QEMU. But we should handle those scenes, meanwhile the device object gained by id is virtio-net-pci device not equals i-dev. 2. not every kinds of device can configure bootindex property, such as usb host adapters. It is a waste and useless for those devices. This is the main reason. I would prefer to waste a few cycles scanning the boot index list every time a device is removed, than risking crashing QEMU in case somebody forget to add a del_boot_device_path() call. OK, fine! Maybe I should do this in device_finalize() as Gerd's previous suggestion, like yours. Thanks. 3. virtio-net device's parent is virtio-pci device, which configured id property, But the device saved in global fw_boot_order list is virtio-net device have not id property. If we put call del_boot_device_path(dev) in virtio_net_device_unrealize we can delete it from fw_boot_order directly. Sorry, I don't understand what you mean here. If virtio-net doesn't have an id property, would the current version of del_boot_device_path() even work? Please see above comments. Thanks for your review! Best regards, -Gonglei
Re: [Qemu-devel] [questions] about KVM asaMicrosoft-compatiblehypervisor
Hi Zhang, No I haven't seen such problem Which kernel version are you running? Host kernel: RHEL7-RC1(linux-3.10.0). Does it include the latest lazy eli changes? lazy eli or lazy eoi? How to confirm whether lazy eli has been included? Btw, hv_spinlocks=0xfff is a pretty huge value. which value do you advise to use? Thanks, Zhang Haoyu Best regards, Vadim.
Re: [Qemu-devel] [PATCH v4 4/8] bootindex: delete bootindex when device is removed
Best regards, -Gonglei -Original Message- From: Eduardo Habkost [mailto:ehabk...@redhat.com] Sent: Friday, August 01, 2014 10:46 PM To: Gonglei (Arei) Cc: qemu-devel@nongnu.org; chenliang (T); Huangweidong (C); m...@redhat.com; a...@ozlabs.ru; hu...@cn.fujitsu.com; arm...@redhat.com; kra...@redhat.com; ak...@redhat.com; ag...@suse.de; aligu...@amazon.com; gaowanl...@cn.fujitsu.com; Luonengjun; Huangpeng (Peter); h...@linux.com; stefa...@redhat.com; pbonz...@redhat.com; lcapitul...@redhat.com; kw...@redhat.com; peter.crosthwa...@xilinx.com; imamm...@redhat.com; afaer...@suse.de Subject: Re: [Qemu-devel] [PATCH v4 4/8] bootindex: delete bootindex when device is removed On Thu, Jul 31, 2014 at 05:47:29PM +0800, arei.gong...@huawei.com wrote: From: Gonglei arei.gong...@huawei.com Device should be removed from global boot list when it is hot-unplugged. Signed-off-by: Chenliang chenlian...@huawei.com Signed-off-by: Gonglei arei.gong...@huawei.com --- hw/block/virtio-blk.c| 1 + hw/i386/kvm/pci-assign.c | 1 + hw/misc/vfio.c | 1 + hw/net/e1000.c | 1 + hw/net/eepro100.c| 1 + hw/net/ne2000.c | 1 + hw/net/rtl8139.c | 1 + hw/net/virtio-net.c | 1 + hw/net/vmxnet3.c | 1 + hw/scsi/scsi-generic.c | 1 + hw/usb/dev-network.c | 1 + hw/usb/host-libusb.c | 1 + hw/usb/redirect.c| 1 + 13 files changed, 13 insertions(+) Grepping for add_boot_device_path, I don't see corresponding del_boot_device_path() calls in this patch for the following: hw/ide/qdev.c:add_boot_device_path(dev-conf.bootindex, dev-qdev, hw/block/fdc.c:add_boot_device_path(isa-bootindexA, dev, /floppy@0); hw/block/fdc.c:add_boot_device_path(isa-bootindexB, dev, /floppy@1); hw/net/pcnet.c:add_boot_device_path(s-conf.bootindex, dev, /ethernet-phy@0); hw/net/spapr_llan.c:add_boot_device_path(dev-nicconf.bootindex, DEVICE(dev), ); hw/scsi/scsi-disk.c:add_boot_device_path(s-qdev.conf.bootindex, dev-qdev, NULL); Why we don't need del_boot_device_path() calls for those? I think those device don't support hot-plug/hot-unplug. So, needn't call del_boot_device_path(). But maybe I make a mistake for pcnet and scsi-disk. I will adopt Gerd's suggestion in v5: call this from device_finalize() instead of placing it into each individual device. Thanks. These seem to be OK, and are handled by this patch: hw/i386/kvm/pci-assign.c:add_boot_device_path(dev-bootindex, pci_dev-qdev, NULL); hw/block/virtio-blk.c:add_boot_device_path(s-conf-bootindex, dev, /disk@0,0); hw/misc/vfio.c:add_boot_device_path(vdev-bootindex, pdev-qdev, NULL); hw/net/rtl8139.c:add_boot_device_path(s-conf.bootindex, d, /ethernet-phy@0); hw/net/e1000.c:add_boot_device_path(d-conf.bootindex, dev, /ethernet-phy@0); hw/net/eepro100.c:add_boot_device_path(s-conf.bootindex, pci_dev-qdev, /ethernet-phy@0); hw/net/ne2000.c:add_boot_device_path(s-c.bootindex, pci_dev-qdev, /ethernet-phy@0); hw/net/virtio-net.c:add_boot_device_path(n-nic_conf.bootindex, dev, /ethernet-phy@0); hw/net/vmxnet3.c:add_boot_device_path(s-conf.bootindex, dev, /ethernet-phy@0); hw/scsi/scsi-generic.c:add_boot_device_path(s-conf.bootindex, s-qdev, NULL); hw/usb/dev-network.c:add_boot_device_path(s-conf.bootindex, dev-qdev, /ethernet@0); hw/usb/host-libusb.c:add_boot_device_path(s-bootindex, udev-qdev, NULL); hw/usb/redirect.c:add_boot_device_path(dev-bootindex, udev-qdev, NULL); This one has dev==NULL, so it looks OK: hw/core/loader.c:add_boot_device_path(bootindex, NULL, devpath); This is modify_boot_device_path(), so it's OK: vl.c:add_boot_device_path(bootindex, dev, old_entry-suffix); -- Eduardo
Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex command
Hi,
Subject: Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex command
On Fri, Jul 25, 2014 at 02:52:50PM +0800, arei.gong...@huawei.com wrote:
From: Gonglei arei.gong...@huawei.com
Adds set-bootindex id=xx,bootindex=xx,suffix=xx QMP command.
Example QMP command:
- { execute: set-bootindex, arguments: { id: ide0-0-1,
bootindex:
1, suffix: /disk@0}}
- { return: {} }
Signed-off-by: Gonglei arei.gong...@huawei.com
Signed-off-by: Chenliang chenlian...@huawei.com
---
qapi-schema.json | 16
qmp-commands.hx | 24
qmp.c| 17 +
3 files changed, 57 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json
index b11aad2..a9ef0be 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1704,6 +1704,22 @@
{ 'command': 'device_del', 'data': {'id': 'str'} }
##
+# @set-bootindex:
+#
+# set bootindex of a devcie
+#
+# @id: the name of the device
+# @bootindex: the bootindex of the device
+# @suffix: #optional a suffix of the device
+#
+# Returns: Nothing on success
+# If @id is not a valid device, DeviceNotFound
+#
+# Since: 2.2
+##
+{ 'command': 'set-bootindex', 'data': {'id': 'str', 'bootindex': 'int',
'*suffix':
'str'} }
+
+##
I wonder if we could simply use qom-set for that. How many devices
actually support having multiple bootindex entries with different
suffixes?
AFAICT, the floppy device support two bootindex with different suffixes.
Best regards,
-Gonglei
Re: [Qemu-devel] [PATCH v4 1/8] bootindex: add modify_boot_device_path function
Hi,
On Thu, Jul 31, 2014 at 05:47:26PM +0800, arei.gong...@huawei.com wrote:
[...]
+void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
+ const char *suffix)
+{
+FWBootEntry *i, *old_entry = NULL;
+
+assert(dev != NULL || suffix != NULL);
+
+if (bootindex = 0) {
+QTAILQ_FOREACH(i, fw_boot_order, link) {
+if (i-bootindex == bootindex) {
+qerror_report(ERROR_CLASS_GENERIC_ERROR,
+ The bootindex %d has already been
used,
+ bootindex);
Isn't an Error** parameter preferable here, instead of using
qerror_report()?
Good catch. I'm not following this series, but using qerror_report() is
almost always wrong these days.
Would you give me some advice? Thanks.
Using error_report() instead of qerror_report()?
Best regards,
-Gonglei
Re: [Qemu-devel] [v2][PATCH 2/5] hw:pci-host:piix: split i440fx_init
On 2014/7/31 17:53, Michael S. Tsirkin wrote:
On Thu, Jul 31, 2014 at 05:26:41PM +0800, Chen, Tiejun wrote:
On 2014/7/31 17:10, Michael S. Tsirkin wrote:
On Thu, Jul 31, 2014 at 02:31:36PM +0800, Tiejun Chen wrote:
We'd like to split i440fx_init and then we can share something
with other stuff.
Signed-off-by: Tiejun Chen tiejun.c...@intel.com
I think this is too much work for very little benefit.
Just pass const char *type to i440fx_init.
You know we will introduce that faked PCIe device represented that PCH
later,
Later when? On top of this patch series? Would like to see it all
before applying this ...
so how to distinguish them? Are you saying I should check the type
like this?
if(Xen-Type)
{}
else
{}
No! Put the code in init function for the respective class,
pass type as an argument:
i440fx: make types configurable at run-time
Xen wants to supply a different pci and host devices,
inheriting i440fx devices. Make types configurable.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Looks this is not enough as well since after we use patch #3 to
introduce that host bridge,
@@ -93,6 +93,9 @@ typedef struct PIIX3State {
#define I440FX_PCI_DEVICE(obj) \
OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
+#define XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE(obj) \
+OBJECT_CHECK(PCII440FXState, (obj),
TYPE_XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE)
+
struct PCII440FXState {
/* private */
PCIDevice parent_obj;
Inside i440fx_init(), we still need to replace that macro,
I440FX_PCI_DEVICE, with XEN_IGD_PASSTHROUGH_I440FX_PCI_DEVICE in xenigd
case.
So looks we'd better split that i440fx_init() to address this point.
Thanks
Tiejun
--
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index be8fdfe..86f295a 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -230,7 +230,11 @@ extern int no_hpet;
struct PCII440FXState;
typedef struct PCII440FXState PCII440FXState;
-PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix_devfn,
+#define TYPE_I440FX_PCI_HOST_BRIDGE i440FX-pcihost
+#define TYPE_I440FX_PCI_DEVICE i440FX
+
+PCIBus *i440fx_init(const char *host_type, const char *pci_type,
+PCII440FXState **pi440fx_state, int *piix_devfn,
ISABus **isa_bus, qemu_irq *pic,
MemoryRegion *address_space_mem,
MemoryRegion *address_space_io,
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 31125b7..e0979cd 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -194,7 +194,9 @@ static void pc_init1(MachineState *machine,
}
if (pci_enabled) {
-pci_bus = i440fx_init(i440fx_state, piix3_devfn, isa_bus, gsi,
+pci_bus = i440fx_init(TYPE_I440FX_PCI_HOST_BRIDGE,
+ TYPE_I440FX_PCI_DEVICE,
+ i440fx_state, piix3_devfn, isa_bus, gsi,
system_memory, system_io, machine-ram_size,
below_4g_mem_size,
above_4g_mem_size,
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index e0e0946..0cd82b8 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -40,7 +40,6 @@
* http://download.intel.com/design/chipsets/datashts/29054901.pdf
*/
-#define TYPE_I440FX_PCI_HOST_BRIDGE i440FX-pcihost
#define I440FX_PCI_HOST_BRIDGE(obj) \
OBJECT_CHECK(I440FXState, (obj), TYPE_I440FX_PCI_HOST_BRIDGE)
@@ -91,7 +90,6 @@ typedef struct PIIX3State {
MemoryRegion rcr_mem;
} PIIX3State;
-#define TYPE_I440FX_PCI_DEVICE i440FX
#define I440FX_PCI_DEVICE(obj) \
OBJECT_CHECK(PCII440FXState, (obj), TYPE_I440FX_PCI_DEVICE)
@@ -305,7 +303,8 @@ static int i440fx_initfn(PCIDevice *dev)
return 0;
}
-PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
+PCIBus *i440fx_init(const char *host_type, const char *pci_type,
+PCII440FXState **pi440fx_state,
int *piix3_devfn,
ISABus **isa_bus, qemu_irq *pic,
MemoryRegion *address_space_mem,
@@ -325,7 +324,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
unsigned i;
I440FXState *i440fx;
-dev = qdev_create(NULL, TYPE_I440FX_PCI_HOST_BRIDGE);
+dev = qdev_create(NULL, host_type);
s = PCI_HOST_BRIDGE(dev);
b = pci_bus_new(dev, NULL, pci_address_space,
address_space_io, 0, TYPE_PCI_BUS);
@@ -333,7 +332,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
object_property_add_child(qdev_get_machine(), i440fx, OBJECT(dev),
NULL);
qdev_init_nofail(dev);
-d = pci_create_simple(b, 0, TYPE_I440FX_PCI_DEVICE);
+d = pci_create_simple(b, 0, pci_type);
*pi440fx_state = I440FX_PCI_DEVICE(d);
f = *pi440fx_state;
f-system_memory = address_space_mem;
[Qemu-devel] [PATCH] configure: replace \n with space in optarg
When optarg happens to contain \n like: ../configure --target-list='i386-softmmu x86_64-softmmu' make will fail with message: config-host.mak:45: *** missing separator. Stop. This patch fix this problem by replacing \n with space in optarg. Signed-off-by: Hu Tao hu...@cn.fujitsu.com --- configure | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure b/configure index f7685b5..0ee9de1 100755 --- a/configure +++ b/configure @@ -338,7 +338,7 @@ numa= # parse CC options first for opt do - optarg=`expr x$opt : 'x[^=]*=\(.*\)'` + optarg=`expr x$opt : 'x[^=]*=\(.*\)' | tr '\n' ' '` case $opt in --cross-prefix=*) cross_prefix=$optarg ;; @@ -722,7 +722,7 @@ fi werror= for opt do - optarg=`expr x$opt : 'x[^=]*=\(.*\)'` + optarg=`expr x$opt : 'x[^=]*=\(.*\)' | tr '\n' ' '` case $opt in --help|-h) show_help=yes ;; -- 1.9.3
Re: [Qemu-devel] [v2 1/3] query-memdev: fix potential memory leaks
On Mon, Aug 04, 2014 at 12:21:17PM +0800, Chen Fan wrote:
Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
Reviewed-by: Hu Tao hu...@cn.fujitsu.com
---
numa.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/numa.c b/numa.c
index 7bf7834..a2b4bca 100644
--- a/numa.c
+++ b/numa.c
@@ -318,10 +318,11 @@ void memory_region_allocate_system_memory(MemoryRegion
*mr, Object *owner,
static int query_memdev(Object *obj, void *opaque)
{
MemdevList **list = opaque;
+MemdevList *m = NULL;
Error *err = NULL;
if (object_dynamic_cast(obj, TYPE_MEMORY_BACKEND)) {
-MemdevList *m = g_malloc0(sizeof(*m));
+m = g_malloc0(sizeof(*m));
m-value = g_malloc0(sizeof(*m-value));
@@ -369,6 +370,9 @@ static int query_memdev(Object *obj, void *opaque)
return 0;
error:
+g_free(m-value);
+g_free(m);
+
return -1;
}
--
1.9.3
Re: [Qemu-devel] [v2 2/3] qom/object.c: fix string_output_get_string() memory leak
On Mon, Aug 04, 2014 at 12:21:18PM +0800, Chen Fan wrote:
string_output_get_string() uses g_string_free(str, false) to
transfer the 'str' pointer to callers and never free it.
Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
Reviewed-by: Hu Tao hu...@cn.fujitsu.com
---
hmp.c| 6 --
qom/object.c | 12 ++--
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/hmp.c b/hmp.c
index 4d1838e..ba40c75 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1687,6 +1687,7 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
MemdevList *memdev_list = qmp_query_memdev(err);
MemdevList *m = memdev_list;
StringOutputVisitor *ov;
+char *str;
int i = 0;
@@ -1704,9 +1705,10 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
m-value-prealloc ? true : false);
monitor_printf(mon, policy: %s\n,
HostMemPolicy_lookup[m-value-policy]);
-monitor_printf(mon, host nodes: %s\n,
- string_output_get_string(ov));
+str = string_output_get_string(ov);
+monitor_printf(mon, host nodes: %s\n, str);
+g_free(str);
string_output_visitor_cleanup(ov);
m = m-next;
i++;
diff --git a/qom/object.c b/qom/object.c
index 0e8267b..e5aed60 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -948,14 +948,18 @@ int object_property_get_enum(Object *obj, const char
*name,
{
StringOutputVisitor *sov;
StringInputVisitor *siv;
+char *str;
int ret;
sov = string_output_visitor_new(false);
object_property_get(obj, string_output_get_visitor(sov), name, errp);
-siv = string_input_visitor_new(string_output_get_string(sov));
+str = string_output_get_string(sov);
+siv = string_input_visitor_new(str);
string_output_visitor_cleanup(sov);
visit_type_enum(string_input_get_visitor(siv),
ret, strings, NULL, name, errp);
+
+g_free(str);
string_input_visitor_cleanup(siv);
return ret;
@@ -966,13 +970,17 @@ void object_property_get_uint16List(Object *obj, const
char *name,
{
StringOutputVisitor *ov;
StringInputVisitor *iv;
+char *str;
ov = string_output_visitor_new(false);
object_property_get(obj, string_output_get_visitor(ov),
name, errp);
-iv = string_input_visitor_new(string_output_get_string(ov));
+str = string_output_get_string(ov);
+iv = string_input_visitor_new(str);
visit_type_uint16List(string_input_get_visitor(iv),
list, NULL, errp);
+
+g_free(str);
string_output_visitor_cleanup(ov);
string_input_visitor_cleanup(iv);
}
--
1.9.3
Re: [Qemu-devel] [v2 3/3] hmp: fix MemdevList memory leak
On Mon, Aug 04, 2014 at 12:21:19PM +0800, Chen Fan wrote:
the memdev_list in hmp_info_memdev() is never freed.
so we use existent method qapi_free_MemdevList() to free it.
and also we can use qapi_free_MemdevList() to replace list loops
to clean up the memdev list in error path.
Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
Reviewed-by: Hu Tao hu...@cn.fujitsu.com
---
hmp.c | 2 ++
numa.c | 9 ++---
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/hmp.c b/hmp.c
index ba40c75..40a90da 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1715,4 +1715,6 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
}
monitor_printf(mon, \n);
+
+qapi_free_MemdevList(memdev_list);
}
diff --git a/numa.c b/numa.c
index a2b4bca..b3e140e 100644
--- a/numa.c
+++ b/numa.c
@@ -379,7 +379,7 @@ error:
MemdevList *qmp_query_memdev(Error **errp)
{
Object *obj;
-MemdevList *list = NULL, *m;
+MemdevList *list = NULL;
obj = object_resolve_path(/objects, NULL);
if (obj == NULL) {
@@ -393,11 +393,6 @@ MemdevList *qmp_query_memdev(Error **errp)
return list;
error:
-while (list) {
-m = list;
-list = list-next;
-g_free(m-value);
-g_free(m);
-}
+qapi_free_MemdevList(list);
return NULL;
}
--
1.9.3
Re: [Qemu-devel] [PATCH v4 1/8] bootindex: add modify_boot_device_path function
Gonglei (Arei) arei.gong...@huawei.com writes:
Hi,
On Thu, Jul 31, 2014 at 05:47:26PM +0800, arei.gong...@huawei.com wrote:
[...]
+void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
+ const char *suffix)
+{
+FWBootEntry *i, *old_entry = NULL;
+
+assert(dev != NULL || suffix != NULL);
+
+if (bootindex = 0) {
+QTAILQ_FOREACH(i, fw_boot_order, link) {
+if (i-bootindex == bootindex) {
+qerror_report(ERROR_CLASS_GENERIC_ERROR,
+ The bootindex %d has already been
used,
+ bootindex);
Isn't an Error** parameter preferable here, instead of using
qerror_report()?
Good catch. I'm not following this series, but using qerror_report() is
almost always wrong these days.
Yes. http://wiki.qemu.org/CodeTransitions#Error_reporting explains:
qerror_report() is a transitional interface to help with converting
existing HMP commands to QMP. It should not be used elsewhere. Use
Error objects instead with error_propagate() and error_setg()
instead.
Would you give me some advice? Thanks.
Using error_report() instead of qerror_report()?
Depends on how the function is used.
If you know this can only run during QEMU startup (e.g. command line
processing) or in a *human* monitor, error_report() is fine.
If the error is propagated up the call chain to some place that reports
it via its Error ** parameter to its caller, then you should consider
passing an Error object created with error_setg() here up the call
chain.
Not the case right now, as your modify_boot_device_path() cannot fail.
Whether that's appropriate I can't tell without examining more of your
patch.
[Qemu-devel] Cc'ing emails [was: [PATCH] dump.c: Fix memory leak issue in cleanup processing for dump_init()]
Please stop Cc'ing me emails sent to, at least, qemu-trivial@. I'm about to filter personal emails which are also sent to some mailinglists I receive. I'd not do that, because this is a good practice to Cc someone like that for various really important or urgent emails, and if I to apply such a filter, these urgent emails will be filtered too, obviously. I'm not sure how people treat these cases or deal with them. We are subscribed to, in particular, qemu-devel@, and active maintainers look there too, so receive more than one copy of many emails. It is becoming worse. With get_maintainer.pl pulling addresses of people who made changes or commits to files by default, contributing to the project becomes a bit dangerous. Because as a result, once you fix something, you're essentially being subscribed to a spam list, because other contributors start Ccing you for the patches with which you have absolutely nothing to do, and if a discussion emerges, you can't opt out of it anymore (especially for patches which raise hot discussions). So I'd rather think twice before contributing anything... Thanks, /mjt
Re: [Qemu-devel] [PATCH v4 1/8] bootindex: add modify_boot_device_path function
Hi,
On Thu, Jul 31, 2014 at 05:47:26PM +0800, arei.gong...@huawei.com
wrote:
[...]
+void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
+ const char *suffix)
+{
+FWBootEntry *i, *old_entry = NULL;
+
+assert(dev != NULL || suffix != NULL);
+
+if (bootindex = 0) {
+QTAILQ_FOREACH(i, fw_boot_order, link) {
+if (i-bootindex == bootindex) {
+qerror_report(ERROR_CLASS_GENERIC_ERROR,
+ The bootindex %d has already
been
used,
+ bootindex);
Isn't an Error** parameter preferable here, instead of using
qerror_report()?
Good catch. I'm not following this series, but using qerror_report() is
almost always wrong these days.
Yes. http://wiki.qemu.org/CodeTransitions#Error_reporting explains:
qerror_report() is a transitional interface to help with converting
existing HMP commands to QMP. It should not be used elsewhere. Use
Error objects instead with error_propagate() and error_setg()
instead.
Would you give me some advice? Thanks.
Using error_report() instead of qerror_report()?
Depends on how the function is used.
If you know this can only run during QEMU startup (e.g. command line
processing) or in a *human* monitor, error_report() is fine.
If the error is propagated up the call chain to some place that reports
it via its Error ** parameter to its caller, then you should consider
passing an Error object created with error_setg() here up the call
chain.
Understood, thank you so much!
error_setg() is the best choice in my case.
Not the case right now, as your modify_boot_device_path() cannot fail.
Whether that's appropriate I can't tell without examining more of your
patch.
Best regards,
-Gonglei
[Qemu-devel] [PATCH 2/3] pc-dimm: check if the value of node property
If user specifies a node number that exceeds the available numa nodes in
emulated system for pc-dimm device, the device will reports an invalid _PXM
to OSPM. Fix it by checking the node value.
Signed-off-by: Hu Tao hu...@cn.fujitsu.com
---
hw/mem/pc-dimm.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 08f49ed..92e276f 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -252,6 +252,11 @@ static void pc_dimm_realize(DeviceState *dev, Error **errp)
error_setg(errp, ' PC_DIMM_MEMDEV_PROP ' property is not set);
return;
}
+if (dimm-node = nb_numa_nodes) {
+error_setg(errp, ' PC_DIMM_NODE_PROP
+ ' exceeds numa node number: % PRId32, nb_numa_nodes);
+return;
+}
}
static MemoryRegion *pc_dimm_get_memory_region(PCDIMMDevice *dimm)
--
1.9.3
[Qemu-devel] [PATCH 1/3] hw:i386: typo fix: MEMORY_HOPTLUG_DEVICE - MEMORY_HOTPLUG_DEVICE
Signed-off-by: Hu Tao hu...@cn.fujitsu.com
---
hw/i386/acpi-dsdt.dsl | 4 ++--
hw/i386/acpi-dsdt.hex.generated | 8
hw/i386/q35-acpi-dsdt.dsl | 4 ++--
hw/i386/ssdt-mem.dsl| 16
hw/i386/ssdt-misc.dsl | 2 +-
include/hw/acpi/pc-hotplug.h| 2 +-
6 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/hw/i386/acpi-dsdt.dsl b/hw/i386/acpi-dsdt.dsl
index 6ba0170..559f4b6 100644
--- a/hw/i386/acpi-dsdt.dsl
+++ b/hw/i386/acpi-dsdt.dsl
@@ -302,7 +302,7 @@ DefinitionBlock (
/
* General purpose events
/
-External(\_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_SCAN_METHOD,
MethodObj)
+External(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_SCAN_METHOD,
MethodObj)
Scope(\_GPE) {
Name(_HID, ACPI0006)
@@ -321,7 +321,7 @@ DefinitionBlock (
}
Method(_E03) {
// Memory hotplug event
-\_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_SCAN_METHOD()
+\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_SCAN_METHOD()
}
Method(_L04) {
}
diff --git a/hw/i386/acpi-dsdt.hex.generated b/hw/i386/acpi-dsdt.hex.generated
index 6c8a1fc..a21bf41 100644
--- a/hw/i386/acpi-dsdt.hex.generated
+++ b/hw/i386/acpi-dsdt.hex.generated
@@ -8,7 +8,7 @@ static unsigned char AcpiDsdtAmlCode[] = {
0x0,
0x0,
0x1,
-0x2e,
+0x1f,
0x42,
0x58,
0x50,
@@ -31,9 +31,9 @@ static unsigned char AcpiDsdtAmlCode[] = {
0x4e,
0x54,
0x4c,
-0x13,
-0x9,
-0x12,
+0x28,
+0x5,
+0x10,
0x20,
0x10,
0x49,
diff --git a/hw/i386/q35-acpi-dsdt.dsl b/hw/i386/q35-acpi-dsdt.dsl
index 8c3eae7..054b035 100644
--- a/hw/i386/q35-acpi-dsdt.dsl
+++ b/hw/i386/q35-acpi-dsdt.dsl
@@ -410,7 +410,7 @@ DefinitionBlock (
/
* General purpose events
/
-External(\_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_SCAN_METHOD,
MethodObj)
+External(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_SCAN_METHOD,
MethodObj)
Scope(\_GPE) {
Name(_HID, ACPI0006)
@@ -425,7 +425,7 @@ DefinitionBlock (
}
Method(_E03) {
// Memory hotplug event
-\_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_SCAN_METHOD()
+\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_SCAN_METHOD()
}
Method(_L04) {
}
diff --git a/hw/i386/ssdt-mem.dsl b/hw/i386/ssdt-mem.dsl
index 8e17bd1..22ff5dd 100644
--- a/hw/i386/ssdt-mem.dsl
+++ b/hw/i386/ssdt-mem.dsl
@@ -39,10 +39,10 @@ ACPI_EXTRACT_ALL_CODE ssdm_mem_aml
DefinitionBlock (ssdt-mem.aml, SSDT, 0x02, BXPC, CSSDT, 0x1)
{
-External(\_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_CRS_METHOD, MethodObj)
-External(\_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_STATUS_METHOD,
MethodObj)
-External(\_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_OST_METHOD, MethodObj)
-External(\_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_PROXIMITY_METHOD,
MethodObj)
+External(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_CRS_METHOD, MethodObj)
+External(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_STATUS_METHOD,
MethodObj)
+External(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_OST_METHOD, MethodObj)
+External(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_PROXIMITY_METHOD,
MethodObj)
Scope(\_SB) {
/* v-- DO NOT EDIT --v */
@@ -58,19 +58,19 @@ DefinitionBlock (ssdt-mem.aml, SSDT, 0x02, BXPC,
CSSDT, 0x1)
Name(_HID, EISAID(PNP0C80))
Method(_CRS, 0) {
-
Return(\_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_CRS_METHOD(_UID))
+
Return(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_CRS_METHOD(_UID))
}
Method(_STA, 0) {
-
Return(\_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_STATUS_METHOD(_UID))
+
Return(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_STATUS_METHOD(_UID))
}
Method(_PXM, 0) {
-
Return(\_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_PROXIMITY_METHOD(_UID))
+
Return(\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_PROXIMITY_METHOD(_UID))
}
Method(_OST, 3) {
-\_SB.PCI0.MEMORY_HOPTLUG_DEVICE.MEMORY_SLOT_OST_METHOD(_UID,
Arg0, Arg1, Arg2)
+\_SB.PCI0.MEMORY_HOTPLUG_DEVICE.MEMORY_SLOT_OST_METHOD(_UID,
Arg0, Arg1, Arg2)
}
}
}
diff --git a/hw/i386/ssdt-misc.dsl b/hw/i386/ssdt-misc.dsl
index d329b8b..0fd4480 100644
--- a/hw/i386/ssdt-misc.dsl
+++ b/hw/i386/ssdt-misc.dsl
@@ -120,7 +120,7 @@ DefinitionBlock (ssdt-misc.aml, SSDT, 0x01, BXPC,
BXSSDTSUSP, 0x1)
External(MEMORY_SLOT_NOTIFY_METHOD, MethodObj)
Scope(\_SB.PCI0) {
-Device(MEMORY_HOPTLUG_DEVICE) {
+
[Qemu-devel] [PATCH 3/3] numa: show hex number in error message for consistency and prefix them with 0x
The error messages before and after patch are:
before:
qemu-system-x86_64: total memory for NUMA nodes (134217728) should equal RAM
size (2000)
after:
qemu-system-x86_64: total memory for NUMA nodes (0x800) should equal RAM
size (0x2000)
Signed-off-by: Hu Tao hu...@cn.fujitsu.com
---
numa.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/numa.c b/numa.c
index 7bf7834..c78cec9 100644
--- a/numa.c
+++ b/numa.c
@@ -210,8 +210,8 @@ void set_numa_nodes(void)
numa_total += numa_info[i].node_mem;
}
if (numa_total != ram_size) {
-error_report(total memory for NUMA nodes (% PRIu64 )
- should equal RAM size ( RAM_ADDR_FMT ),
+error_report(total memory for NUMA nodes (0x% PRIx64 )
+ should equal RAM size (0x RAM_ADDR_FMT ),
numa_total, ram_size);
exit(1);
}
--
1.9.3
[Qemu-devel] [PATCH 0/3] some numa memory related fixes
See each patch for the detail. Hu Tao (3): hw:i386: typo fix: MEMORY_HOPTLUG_DEVICE - MEMORY_HOTPLUG_DEVICE pc-dimm: check if node property exceeds available numa nodes numa: show hex number in error message for consistency and prefix them with 0x hw/i386/acpi-dsdt.dsl | 4 ++-- hw/i386/acpi-dsdt.hex.generated | 8 hw/i386/q35-acpi-dsdt.dsl | 4 ++-- hw/i386/ssdt-mem.dsl| 16 hw/i386/ssdt-misc.dsl | 2 +- hw/mem/pc-dimm.c| 5 + include/hw/acpi/pc-hotplug.h| 2 +- numa.c | 4 ++-- 8 files changed, 25 insertions(+), 20 deletions(-) -- 1.9.3
Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex command
Gonglei (Arei) arei.gong...@huawei.com writes:
Hi,
Subject: Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex command
On Fri, Jul 25, 2014 at 02:52:50PM +0800, arei.gong...@huawei.com wrote:
From: Gonglei arei.gong...@huawei.com
Adds set-bootindex id=xx,bootindex=xx,suffix=xx QMP command.
Example QMP command:
- { execute: set-bootindex, arguments: { id: ide0-0-1,
bootindex:
1, suffix: /disk@0}}
- { return: {} }
Signed-off-by: Gonglei arei.gong...@huawei.com
Signed-off-by: Chenliang chenlian...@huawei.com
---
qapi-schema.json | 16
qmp-commands.hx | 24
qmp.c| 17 +
3 files changed, 57 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json
index b11aad2..a9ef0be 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1704,6 +1704,22 @@
{ 'command': 'device_del', 'data': {'id': 'str'} }
##
+# @set-bootindex:
+#
+# set bootindex of a devcie
+#
+# @id: the name of the device
+# @bootindex: the bootindex of the device
+# @suffix: #optional a suffix of the device
+#
+# Returns: Nothing on success
+# If @id is not a valid device, DeviceNotFound
+#
+# Since: 2.2
+##
+{ 'command': 'set-bootindex', 'data': {'id': 'str', 'bootindex':
int', '*suffix':
'str'} }
+
+##
I wonder if we could simply use qom-set for that. How many devices
actually support having multiple bootindex entries with different
suffixes?
AFAICT, the floppy device support two bootindex with different suffixes.
Block device models commonly a single block device. By convention,
property drive is the backend, and property bootindex the bootindex,
if the device model supports that.
The device ID suffices to identify a block device for such device
models.
The floppy device model is an exception. It folds multiple real-world
objects into one: the controller and the actual drives. Have a look at
-device isa-fdc,help:
isa-fdc.iobase=uint32
isa-fdc.irq=uint32
isa-fdc.dma=uint32
isa-fdc.driveA=drive
isa-fdc.driveB=drive
isa-fdc.bootindexA=int32
isa-fdc.bootindexB=int32
isa-fdc.check_media_rate=on/off
The properties ending with 'A' or 'B' apply to the first and the second
drive, the others to the controller.
Unfortunately, this means the device ID by itself doesn't identify the
floppy device.
Arguably, this is lousy modeling --- we really should model separate
real-world objects as separate objects. But making floppies pretty (or
even sane) isn't anyone's priority nowadays.
Since qom-set works on *properties*, I can't see why it couldn't be used
for changing bootindexes. There is no ambiguity even with floppy.
You either set bootindexA or bootindexB. No need to fuzz around with
suffixes. Or am I missing something?
Re: [Qemu-devel] [PATCH 3/4] libqos: add a simple first-fit memory allocator
Stefan Hajnoczi stefa...@gmail.com writes:
On Thu, Jul 31, 2014 at 05:14:12PM -0400, John Snow wrote:
On 07/31/2014 06:13 AM, Stefan Hajnoczi wrote:
On Wed, Jul 30, 2014 at 06:28:28PM -0400, John Snow wrote:
-static uint64_t pc_alloc(QGuestAllocator *allocator, size_t size)
+static inline void mlist_insert(MemList *head, MemBlock *insr)
{
-PCAlloc *s = container_of(allocator, PCAlloc, alloc);
-uint64_t addr;
+QTAILQ_INSERT_HEAD(head, insr, MLIST_ENTNAME);
+}
+
+static inline void mlist_append(MemList *head, MemBlock *node)
+{
+QTAILQ_INSERT_TAIL(head, node, MLIST_ENTNAME);
+}
+
+static inline void mlist_unlink(MemList *head, MemBlock *rem)
+{
+QTAILQ_REMOVE(head, rem, MLIST_ENTNAME);
+}
For the same reasons as my comments about the macros, these trivial
functions
are boilerplate. Why not use the QTAILQ macros directly?
For /at least/ the append and insert cases, I desired call-by-value
semantics.
As a matter of taste, I find macros annoying for the reason that you cannot
inline things such as:
mlist_insert(list, mlist_new(...));
but unlink is certainly superfluous, and just something I did for some
consistency.
If there is a matter of style where in-line function call is to be avoided
entirely, I'll just nix all of these trivial inlines.
As a reviewer I prefer to see familiar APIs rather than a convenience
layer because it's extra stuff I need to keep in mind. Sticking to the
APIs makes it quicker for other QEMU developers to parse the code.
Seconded.
That said, feel free to keep the functions if you want.
Can't say, as I haven't studied the patch in detail.
[Qemu-devel] [PATCH 3/4] virtio-blk: fix reference a pointer which might be freed
In function virtio_blk_handle_request, it may freed memory pointed by req,
So do not access member of req after calling this function.
Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com
---
hw/block/virtio-blk.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index c241c50..54a853a 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -458,7 +458,7 @@ static void virtio_blk_handle_output(VirtIODevice *vdev,
VirtQueue *vq)
static void virtio_blk_dma_restart_bh(void *opaque)
{
VirtIOBlock *s = opaque;
-VirtIOBlockReq *req = s-rq;
+VirtIOBlockReq *req = s-rq, *next = NULL;
MultiReqBuffer mrb = {
.num_writes = 0,
};
@@ -469,8 +469,9 @@ static void virtio_blk_dma_restart_bh(void *opaque)
s-rq = NULL;
while (req) {
+next = req-next;
virtio_blk_handle_request(req, mrb);
-req = req-next;
+req = next;
}
virtio_submit_multiwrite(s-bs, mrb);
--
1.7.12.4
[Qemu-devel] [PATCH 2/4] monitor: fix access freed memory
The function monitor_fdset_dup_fd_find_remove() references member of 'mon_fdset'
which may be freed in function monitor_fdset_cleanup()
Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com
---
monitor.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/monitor.c b/monitor.c
index 5bc70a6..41e46a6 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2532,8 +2532,10 @@ static int monitor_fdset_dup_fd_find_remove(int dup_fd,
bool remove)
{
MonFdset *mon_fdset;
MonFdsetFd *mon_fdset_fd_dup;
+int64_t id = -1;
QLIST_FOREACH(mon_fdset, mon_fdsets, next) {
+id = mon_fdset-id;
QLIST_FOREACH(mon_fdset_fd_dup, mon_fdset-dup_fds, next) {
if (mon_fdset_fd_dup-fd == dup_fd) {
if (remove) {
@@ -2542,7 +2544,7 @@ static int monitor_fdset_dup_fd_find_remove(int dup_fd,
bool remove)
monitor_fdset_cleanup(mon_fdset);
}
}
-return mon_fdset-id;
+return id;
}
}
}
--
1.7.12.4
[Qemu-devel] [PATCH 1/4] l2cap: fix access freed memory
Pointer 'ch' will be used in function 'l2cap_channel_open_req_msg' after
it was previously freed in 'l2cap_channel_open'.
Assigned it to NULL after it is freed.
Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com
---
hw/bt/l2cap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/bt/l2cap.c b/hw/bt/l2cap.c
index 2301d6f..591e047 100644
--- a/hw/bt/l2cap.c
+++ b/hw/bt/l2cap.c
@@ -429,7 +429,7 @@ static struct l2cap_chan_s *l2cap_channel_open(struct
l2cap_instance_s *l2cap,
status = L2CAP_CS_NO_INFO;
} else {
g_free(ch);
-
+ch = NULL;
result = L2CAP_CR_NO_MEM;
status = L2CAP_CS_NO_INFO;
}
--
1.7.12.4
[Qemu-devel] [PATCH 4/4] ivshmem: check the value returned by fstat()
The function fstat() may fail, so check its return value.
Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com
---
hw/misc/ivshmem.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 768e528..2667e9f 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -324,7 +324,10 @@ static int check_shm_size(IVShmemState *s, int fd) {
struct stat buf;
-fstat(fd, buf);
+if (fstat(fd, buf) 0) {
+fprintf(stderr, Cannot stat IVSHMEM: %s\n, strerror(errno));
+return -1;
+}
if (s-ivshmem_size buf.st_size) {
fprintf(stderr,
--
1.7.12.4
[Qemu-devel] [PATCH 0/4] fix several bugs about use-after-free and an api abuse
zhanghailiang (4): l2cap: fix access freed memory monitor: fix access freed memory virtio-blk: fix reference a pointer which might be freed ivshmem: check the value returned by fstat() hw/block/virtio-blk.c | 5 +++-- hw/bt/l2cap.c | 2 +- hw/misc/ivshmem.c | 5 - monitor.c | 4 +++- 4 files changed, 11 insertions(+), 5 deletions(-) -- 1.7.12.4
Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex command
Hi,
Subject: Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex command
On Fri, Jul 25, 2014 at 02:52:50PM +0800, arei.gong...@huawei.com wrote:
From: Gonglei arei.gong...@huawei.com
Adds set-bootindex id=xx,bootindex=xx,suffix=xx QMP command.
Example QMP command:
- { execute: set-bootindex, arguments: { id: ide0-0-1,
bootindex:
1, suffix: /disk@0}}
- { return: {} }
Signed-off-by: Gonglei arei.gong...@huawei.com
Signed-off-by: Chenliang chenlian...@huawei.com
---
qapi-schema.json | 16
qmp-commands.hx | 24
qmp.c| 17 +
3 files changed, 57 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json
index b11aad2..a9ef0be 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1704,6 +1704,22 @@
{ 'command': 'device_del', 'data': {'id': 'str'} }
##
+# @set-bootindex:
+#
+# set bootindex of a devcie
+#
+# @id: the name of the device
+# @bootindex: the bootindex of the device
+# @suffix: #optional a suffix of the device
+#
+# Returns: Nothing on success
+# If @id is not a valid device, DeviceNotFound
+#
+# Since: 2.2
+##
+{ 'command': 'set-bootindex', 'data': {'id': 'str', 'bootindex':
int', '*suffix':
'str'} }
+
+##
I wonder if we could simply use qom-set for that. How many devices
actually support having multiple bootindex entries with different
suffixes?
AFAICT, the floppy device support two bootindex with different suffixes.
Block device models commonly a single block device. By convention,
property drive is the backend, and property bootindex the bootindex,
if the device model supports that.
The device ID suffices to identify a block device for such device
models.
The floppy device model is an exception. It folds multiple real-world
objects into one: the controller and the actual drives. Have a look at
-device isa-fdc,help:
isa-fdc.iobase=uint32
isa-fdc.irq=uint32
isa-fdc.dma=uint32
isa-fdc.driveA=drive
isa-fdc.driveB=drive
isa-fdc.bootindexA=int32
isa-fdc.bootindexB=int32
isa-fdc.check_media_rate=on/off
The properties ending with 'A' or 'B' apply to the first and the second
drive, the others to the controller.
Unfortunately, this means the device ID by itself doesn't identify the
floppy device.
Yes.
Arguably, this is lousy modeling --- we really should model separate
real-world objects as separate objects. But making floppies pretty (or
even sane) isn't anyone's priority nowadays.
Hmm... Agreed.
Since qom-set works on *properties*, I can't see why it couldn't be used
for changing bootindexes. There is no ambiguity even with floppy.
Sorry?
You either set bootindexA or bootindexB. No need to fuzz around with
suffixes. Or am I missing something?
Your mean that we just need to think about change one bootindex? Either
set bootindexA or bootindexB, do not need pass suffix for identifying?
Best regards,
-Gonglei
[Qemu-devel] Question of emulation on MSR's in KVM-mode
Hi I'm working on an extension to QEMU (target i386). This involves adding new MSR's. I've got it working in non-KVM mode by adding these MSR's to the state and adding extra cases to helper_wrmsr(), helper_rdmsr(). The guest can now read/write these MSR's as expected. However, it fails when running in KVM-mode. Specifically, writing the MSR's causes GPF. Note that these MSR's are not natively supported by the host CPU. I don't know enough about Intel's VMX to tell if it is even reasonable to expect that this could work for a non-natively supported MSR. As far as I can read in the VMX documentation, the hypervisor can setup a bitmap of which MSR's should cause trap's to the hypervisor and which shouldn't. I guess it would be the KVM kernel module that does this based on input it receives from QEMU. But I haven't been able to find the part of QEMU that negotiates this. I guess the solution for me is to set the necessary bits to that access to the new MSR's causes traps. Next, I need to add/modify the trap handler so that it can handle the MSR's. I would much appreciate any help. Thanks!
Re: [Qemu-devel] fpu/softfloat.c licensing
On 4 August 2014 05:55, Alexey Kardashevskiy a...@ozlabs.ru wrote: Our lawyers refused to provide any public advise on this :-/ Is that it, end of story? :) Well, I 'd still like to get the license fixed to at least my personal satisfaction, but if your lawyers won't confirm whether what we propose will work for them then there is simply no possible way that we can guarantee to make them happy. thanks -- PMM
Re: [Qemu-devel] [PATCH v4] aarch64: Allow -kernel option to take a gzip-compressed kernel.
On Mon, Aug 04, 2014 at 09:05:39AM +1000, Peter Crosthwaite wrote: On Sun, Aug 3, 2014 at 1:45 AM, Richard W.M. Jones rjo...@redhat.com wrote: +max_bytes = UBOOT_MAX_GUNZIP_BYTES; Why does u-boot's maximum size limit apply here? We need some maximum to prevent people uploading a kernel (perhaps from an untrusted source) which is some sort of malicious gzip file that expands to a huge size. In this case the u-boot limit is 64 MB which is larger than most possible kernels, so it seemed like a reasonable limit to choose. You're right there is no connection to u-boot, except that both the -kernel option and u-boot have similar concerns with maximum kernel size, and presumably the u-boot limit is battle-tested. I'll split the patch into two and send v5 soon. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Re: [Qemu-devel] [PATCH 1/4] l2cap: fix access freed memory
zhanghailiang writes:
Pointer 'ch' will be used in function 'l2cap_channel_open_req_msg' after
it was previously freed in 'l2cap_channel_open'.
Assigned it to NULL after it is freed.
Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com
---
hw/bt/l2cap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/bt/l2cap.c b/hw/bt/l2cap.c
index 2301d6f..591e047 100644
--- a/hw/bt/l2cap.c
+++ b/hw/bt/l2cap.c
@@ -429,7 +429,7 @@ static struct l2cap_chan_s *l2cap_channel_open(struct
l2cap_instance_s *l2cap,
status = L2CAP_CS_NO_INFO;
} else {
g_free(ch);
-
+ch = NULL;
result = L2CAP_CR_NO_MEM;
status = L2CAP_CS_NO_INFO;
}
Reviewed-by: Alex Bennée alex.ben...@linaro.org
--
Alex Bennée
Re: [Qemu-devel] [PATCH] configure: replace \n with space in optarg
On 4 August 2014 08:12, Hu Tao hu...@cn.fujitsu.com wrote: When optarg happens to contain \n like: ../configure --target-list='i386-softmmu x86_64-softmmu' make will fail with message: config-host.mak:45: *** missing separator. Stop. Why would you put newlines in the option string? This is likely to break for many configure arguments, not just this one... I'm a bit dubious about taking this change unless you can point to existing practice (eg do autoconf configure scripts clean up whitespace like this?) thanks -- PMM
Re: [Qemu-devel] [PATCH v4] aarch64: Allow -kernel option to take a gzip-compressed kernel.
On 4 August 2014 09:48, Richard W.M. Jones rjo...@redhat.com wrote: On Mon, Aug 04, 2014 at 09:05:39AM +1000, Peter Crosthwaite wrote: On Sun, Aug 3, 2014 at 1:45 AM, Richard W.M. Jones rjo...@redhat.com wrote: +max_bytes = UBOOT_MAX_GUNZIP_BYTES; Why does u-boot's maximum size limit apply here? We need some maximum to prevent people uploading a kernel (perhaps from an untrusted source) which is some sort of malicious gzip file that expands to a huge size. If we care about malicious zipfiles we should probably fix the bits in gunzip() which trust the gzip header more than they should... thanks -- PMM
Re: [Qemu-devel] [PATCH 2/4] monitor: fix access freed memory
zhanghailiang writes:
The function monitor_fdset_dup_fd_find_remove() references member of
'mon_fdset'
which may be freed in function monitor_fdset_cleanup()
Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com
---
monitor.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/monitor.c b/monitor.c
index 5bc70a6..41e46a6 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2532,8 +2532,10 @@ static int monitor_fdset_dup_fd_find_remove(int
dup_fd, bool remove)
{
MonFdset *mon_fdset;
MonFdsetFd *mon_fdset_fd_dup;
+int64_t id = -1;
QLIST_FOREACH(mon_fdset, mon_fdsets, next) {
+id = mon_fdset-id;
QLIST_FOREACH(mon_fdset_fd_dup, mon_fdset-dup_fds, next) {
if (mon_fdset_fd_dup-fd == dup_fd) {
if (remove) {
@@ -2542,7 +2544,7 @@ static int monitor_fdset_dup_fd_find_remove(int dup_fd,
bool remove)
monitor_fdset_cleanup(mon_fdset);
}
}
-return mon_fdset-id;
+return id;
}
}
}
If monitor_fdset_cleanup closes the FD won't you now be passing an
invalid fd to the calling function?
--
Alex Bennée
Re: [Qemu-devel] [PATCH v4] aarch64: Allow -kernel option to take a gzip-compressed kernel.
On Mon, Aug 4, 2014 at 6:48 PM, Richard W.M. Jones rjo...@redhat.com wrote: On Mon, Aug 04, 2014 at 09:05:39AM +1000, Peter Crosthwaite wrote: On Sun, Aug 3, 2014 at 1:45 AM, Richard W.M. Jones rjo...@redhat.com wrote: +max_bytes = UBOOT_MAX_GUNZIP_BYTES; Why does u-boot's maximum size limit apply here? We need some maximum to prevent people uploading a kernel (perhaps from an untrusted source) which is some sort of malicious gzip file that expands to a huge size. In this case the u-boot limit is 64 MB which is larger than most possible kernels, so it seemed like a reasonable limit to choose. Ok. If you really do need this artificial limit then I think you should just make your own macro with the same value. Regards, Peter You're right there is no connection to u-boot, except that both the -kernel option and u-boot have similar concerns with maximum kernel size, and presumably the u-boot limit is battle-tested. I'll split the patch into two and send v5 soon. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Re: [Qemu-devel] [v2 2/3] qom/object.c: fix string_output_get_string() memory leak
On Mon, Aug 4, 2014 at 2:21 PM, Chen Fan chen.fan.f...@cn.fujitsu.com wrote:
string_output_get_string() uses g_string_free(str, false) to
transfer the 'str' pointer to callers and never free it.
Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---
hmp.c| 6 --
qom/object.c | 12 ++--
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/hmp.c b/hmp.c
index 4d1838e..ba40c75 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1687,6 +1687,7 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
MemdevList *memdev_list = qmp_query_memdev(err);
MemdevList *m = memdev_list;
StringOutputVisitor *ov;
+char *str;
int i = 0;
@@ -1704,9 +1705,10 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
m-value-prealloc ? true : false);
monitor_printf(mon, policy: %s\n,
HostMemPolicy_lookup[m-value-policy]);
-monitor_printf(mon, host nodes: %s\n,
- string_output_get_string(ov));
+str = string_output_get_string(ov);
+monitor_printf(mon, host nodes: %s\n, str);
+g_free(str);
string_output_visitor_cleanup(ov);
m = m-next;
i++;
diff --git a/qom/object.c b/qom/object.c
index 0e8267b..e5aed60 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -948,14 +948,18 @@ int object_property_get_enum(Object *obj, const char
*name,
{
StringOutputVisitor *sov;
StringInputVisitor *siv;
+char *str;
int ret;
sov = string_output_visitor_new(false);
object_property_get(obj, string_output_get_visitor(sov), name, errp);
-siv = string_input_visitor_new(string_output_get_string(sov));
+str = string_output_get_string(sov);
+siv = string_input_visitor_new(str);
string_output_visitor_cleanup(sov);
visit_type_enum(string_input_get_visitor(siv),
ret, strings, NULL, name, errp);
+
+g_free(str);
string_input_visitor_cleanup(siv);
return ret;
@@ -966,13 +970,17 @@ void object_property_get_uint16List(Object *obj, const
char *name,
{
StringOutputVisitor *ov;
StringInputVisitor *iv;
+char *str;
ov = string_output_visitor_new(false);
object_property_get(obj, string_output_get_visitor(ov),
name, errp);
-iv = string_input_visitor_new(string_output_get_string(ov));
+str = string_output_get_string(ov);
+iv = string_input_visitor_new(str);
visit_type_uint16List(string_input_get_visitor(iv),
list, NULL, errp);
+
+g_free(str);
string_output_visitor_cleanup(ov);
string_input_visitor_cleanup(iv);
}
--
1.9.3
Re: [Qemu-devel] [PATCH v2 1/7] block: Add status callback to bdrv_amend_options()
The Saturday 02 Aug 2014 à 01:49:15 (+0200), Max Reitz wrote :
Depending on the changed options and the image format,
bdrv_amend_options() may take a significant amount of time. In these
cases, a way to be informed about the operation's status is desirable.
Since the operation is rather complex and may fundamentally change the
image, implementing it as AIO or a couroutine does not seem feasible. On
the other hand, implementing it as a block job would be significantly
more difficult than a simple callback and would not add benefits other
than progress report to the amending operation, because it should not
actually be run as a block job at all.
A callback may not be very pretty, but it's very easy to implement and
perfectly fits its purpose here.
Signed-off-by: Max Reitz mre...@redhat.com
---
block.c | 5 +++--
block/qcow2.c | 3 ++-
include/block/block.h | 8 +++-
include/block/block_int.h | 3 ++-
qemu-img.c| 2 +-
5 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/block.c b/block.c
index 3e252a2..4c8d4d8 100644
--- a/block.c
+++ b/block.c
@@ -5708,12 +5708,13 @@ void bdrv_add_before_write_notifier(BlockDriverState
*bs,
notifier_with_return_list_add(bs-before_write_notifiers, notifier);
}
-int bdrv_amend_options(BlockDriverState *bs, QemuOpts *opts)
+int bdrv_amend_options(BlockDriverState *bs, QemuOpts *opts,
+ BlockDriverAmendStatusCB *status_cb)
{
if (!bs-drv-bdrv_amend_options) {
return -ENOTSUP;
}
-return bs-drv-bdrv_amend_options(bs, opts);
+return bs-drv-bdrv_amend_options(bs, opts, status_cb);
}
/* This function will be called by the bdrv_recurse_is_first_non_filter
method
diff --git a/block/qcow2.c b/block/qcow2.c
index b0faa69..a3a7efc 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2210,7 +2210,8 @@ static int qcow2_downgrade(BlockDriverState *bs, int
target_version)
return 0;
}
-static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts)
+static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
+ BlockDriverAmendStatusCB *status_cb)
{
BDRVQcowState *s = bs-opaque;
int old_version = s-qcow_version, new_version = old_version;
diff --git a/include/block/block.h b/include/block/block.h
index 32d3676..3ce3076 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -309,7 +309,13 @@ typedef enum {
int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode
fix);
-int bdrv_amend_options(BlockDriverState *bs_new, QemuOpts *opts);
+/* The units of offset and total_work_size may be chosen arbitrarily by the
+ * block driver; total_work_size may change during the course of the
amendment
+ * operation */
+typedef void BlockDriverAmendStatusCB(BlockDriverState *bs, int64_t offset,
+ int64_t total_work_size);
+int bdrv_amend_options(BlockDriverState *bs_new, QemuOpts *opts,
+ BlockDriverAmendStatusCB *status_cb);
/* external snapshots */
bool bdrv_recurse_is_first_non_filter(BlockDriverState *bs,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index f6c3bef..5c5798d 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -228,7 +228,8 @@ struct BlockDriver {
int (*bdrv_check)(BlockDriverState* bs, BdrvCheckResult *result,
BdrvCheckMode fix);
-int (*bdrv_amend_options)(BlockDriverState *bs, QemuOpts *opts);
+int (*bdrv_amend_options)(BlockDriverState *bs, QemuOpts *opts,
+ BlockDriverAmendStatusCB *status_cb);
void (*bdrv_debug_event)(BlockDriverState *bs, BlkDebugEvent event);
diff --git a/qemu-img.c b/qemu-img.c
index ef74ae4..90d6b79 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2827,7 +2827,7 @@ static int img_amend(int argc, char **argv)
goto out;
}
-ret = bdrv_amend_options(bs, opts);
+ret = bdrv_amend_options(bs, opts, NULL);
if (ret 0) {
error_report(Error while amending options: %s, strerror(-ret));
goto out;
--
2.0.3
Reviewed-by: Benoit Canet ben...@irqsave.net
[Qemu-devel] [PATCH v4 6/8] spice: don't use 'Yoda conditions'
From: Gonglei arei.gong...@huawei.com
imitate nearby code about using '!value' or 'value == NULL'
Signed-off-by: Gonglei arei.gong...@huawei.com
---
ui/spice-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 7bb91e6..1a2fb4b 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -677,7 +677,7 @@ void qemu_spice_init(void)
if (tls_port) {
x509_dir = qemu_opt_get(opts, x509-dir);
-if (NULL == x509_dir) {
+if (!x509_dir) {
x509_dir = .;
}
@@ -803,7 +803,7 @@ void qemu_spice_init(void)
seamless_migration = qemu_opt_get_bool(opts, seamless-migration, 0);
spice_server_set_seamless_migration(spice_server, seamless_migration);
-if (0 != spice_server_init(spice_server, core_interface)) {
+if (spice_server_init(spice_server, core_interface) != 0) {
error_report(failed to initialize spice server);
exit(1);
};
--
1.7.12.4
[Qemu-devel] [PATCH v4 7/8] vl: don't use 'Yoda conditions'
From: Gonglei arei.gong...@huawei.com
imitate nearby code about using '!value' or 'value == NULL'
Signed-off-by: Gonglei arei.gong...@huawei.com
---
vl.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/vl.c b/vl.c
index fe451aa..04c5abd 100644
--- a/vl.c
+++ b/vl.c
@@ -1136,7 +1136,7 @@ static int drive_init_func(QemuOpts *opts, void *opaque)
static int drive_enable_snapshot(QemuOpts *opts, void *opaque)
{
-if (NULL == qemu_opt_get(opts, snapshot)) {
+if (qemu_opt_get(opts, snapshot) == NULL) {
qemu_opt_set(opts, snapshot, on);
}
return 0;
@@ -2488,8 +2488,9 @@ static int foreach_device_config(int type, int
(*func)(const char *cmdline))
loc_push_restore(conf-loc);
rc = func(conf-cmdline);
loc_pop(conf-loc);
-if (0 != rc)
+if (rc) {
return rc;
+}
}
return 0;
}
--
1.7.12.4
[Qemu-devel] [PATCH v4 5/8] don't use 'Yoda conditions'
From: Gonglei arei.gong...@huawei.com
imitate nearby code about using '!value' or 'value == NULL'
Signed-off-by: Gonglei arei.gong...@huawei.com
---
qdev-monitor.c | 2 +-
qemu-char.c | 2 +-
util/qemu-sockets.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/qdev-monitor.c b/qdev-monitor.c
index f87f3d8..81a4e9b 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -694,7 +694,7 @@ void qmp_device_del(const char *id, Error **errp)
DeviceState *dev;
dev = qdev_find_recursive(sysbus_get_default(), id);
-if (NULL == dev) {
+if (!dev) {
error_set(errp, QERR_DEVICE_NOT_FOUND, id);
return;
}
diff --git a/qemu-char.c b/qemu-char.c
index 956be49..70d5a64 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4117,7 +4117,7 @@ void qmp_chardev_remove(const char *id, Error **errp)
CharDriverState *chr;
chr = qemu_chr_find(id);
-if (NULL == chr) {
+if (chr == NULL) {
error_setg(errp, Chardev '%s' not found, id);
return;
}
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 74cf078..5d38395 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -732,7 +732,7 @@ int unix_connect_opts(QemuOpts *opts, Error **errp,
ConnectState *connect_state = NULL;
int sock, rc;
-if (NULL == path) {
+if (path == NULL) {
error_setg(errp, unix connect: no path specified);
return -1;
}
--
1.7.12.4
[Qemu-devel] [PATCH v4 3/8] audio: don't use 'Yoda conditions'
From: Gonglei arei.gong...@huawei.com
imitate nearby code about using '!value' or 'value == NULL'
Signed-off-by: Gonglei arei.gong...@huawei.com
---
hw/audio/gus.c | 2 +-
hw/audio/hda-codec.c | 3 ++-
hw/audio/sb16.c | 6 +++---
3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/hw/audio/gus.c b/hw/audio/gus.c
index bba6840..4a43ce7 100644
--- a/hw/audio/gus.c
+++ b/hw/audio/gus.c
@@ -212,7 +212,7 @@ static int GUS_read_DMA (void *opaque, int nchan, int
dma_pos, int dma_len)
pos += copied;
}
-if (0 == ((mode 4) 1)) {
+if (((mode 4) 1) == 0) {
DMA_release_DREQ (s-emu.gusdma);
}
return dma_len;
diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
index cbcf521..3c03ff5 100644
--- a/hw/audio/hda-codec.c
+++ b/hw/audio/hda-codec.c
@@ -489,8 +489,9 @@ static int hda_audio_init(HDACodecDevice *hda, const struct
desc_codec *desc)
for (i = 0; i a-desc-nnodes; i++) {
node = a-desc-nodes + i;
param = hda_codec_find_param(node, AC_PAR_AUDIO_WIDGET_CAP);
-if (NULL == param)
+if (param == NULL) {
continue;
+}
type = (param-val AC_WCAP_TYPE) AC_WCAP_TYPE_SHIFT;
switch (type) {
case AC_WID_AUD_OUT:
diff --git a/hw/audio/sb16.c b/hw/audio/sb16.c
index 60c4b3b..bda26d0 100644
--- a/hw/audio/sb16.c
+++ b/hw/audio/sb16.c
@@ -928,7 +928,7 @@ static IO_WRITE_PROTO (dsp_write)
/* if (s-highspeed) */
/* break; */
-if (0 == s-needed_bytes) {
+if (s-needed_bytes == 0) {
command (s, val);
#if 0
if (0 == s-needed_bytes) {
@@ -1212,7 +1212,7 @@ static int SB_read_DMA (void *opaque, int nchan, int
dma_pos, int dma_len)
#endif
if (till = copy) {
-if (0 == s-dma_auto) {
+if (s-dma_auto == 0) {
copy = till;
}
}
@@ -1224,7 +1224,7 @@ static int SB_read_DMA (void *opaque, int nchan, int
dma_pos, int dma_len)
if (s-left_till_irq = 0) {
s-mixer_regs[0x82] |= (nchan 4) ? 2 : 1;
qemu_irq_raise (s-pic);
-if (0 == s-dma_auto) {
+if (s-dma_auto == 0) {
control (s, 0);
speaker (s, 0);
}
--
1.7.12.4
[Qemu-devel] [PATCH v4 8/8] vmxnet3: don't use 'Yoda conditions'
From: Gonglei arei.gong...@huawei.com
imitate nearby code about using '!value' or 'value == NULL'
Signed-off-by: Gonglei arei.gong...@huawei.com
---
hw/net/vmxnet3.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 77bea6f..588149d 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -1009,7 +1009,7 @@ vmxnet3_indicate_packet(VMXNET3State *s)
vmxnet3_dump_rx_descr(rxd);
-if (0 != ready_rxcd_pa) {
+if (ready_rxcd_pa != 0) {
cpu_physical_memory_write(ready_rxcd_pa, rxcd, sizeof(rxcd));
}
@@ -1020,7 +1020,7 @@ vmxnet3_indicate_packet(VMXNET3State *s)
rxcd.gen = new_rxcd_gen;
rxcd.rqID = RXQ_IDX + rx_ridx * s-rxq_num;
-if (0 == bytes_left) {
+if (bytes_left == 0) {
vmxnet3_rx_update_descr(s-rx_pkt, rxcd);
}
@@ -1038,16 +1038,16 @@ vmxnet3_indicate_packet(VMXNET3State *s)
num_frags++;
}
-if (0 != ready_rxcd_pa) {
+if (ready_rxcd_pa != 0) {
rxcd.eop = 1;
-rxcd.err = (0 != bytes_left);
+rxcd.err = (bytes_left != 0);
cpu_physical_memory_write(ready_rxcd_pa, rxcd, sizeof(rxcd));
/* Flush RX descriptor changes */
smp_wmb();
}
-if (0 != new_rxcd_pa) {
+if (new_rxcd_pa != 0) {
vmxnet3_revert_rxc_descr(s, RXQ_IDX);
}
@@ -1190,8 +1190,8 @@ static void vmxnet3_update_mcast_filters(VMXNET3State *s)
s-mcast_list_len = list_bytes / sizeof(s-mcast_list[0]);
s-mcast_list = g_realloc(s-mcast_list, list_bytes);
-if (NULL == s-mcast_list) {
-if (0 == s-mcast_list_len) {
+if (!s-mcast_list) {
+if (s-mcast_list_len == 0) {
VMW_CFPRN(Current multicast list is empty);
} else {
VMW_ERPRN(Failed to allocate multicast list of %d elements,
@@ -1667,7 +1667,7 @@ vmxnet3_io_bar1_write(void *opaque,
* memory address. We save it to temp variable and set the
* shared address only after we get the high part
*/
-if (0 == val) {
+if (val == 0) {
s-device_active = false;
}
s-temp_shared_guest_driver_memory = val;
--
1.7.12.4
[Qemu-devel] [PATCH v4 1/8] CODING_STYLE: Section about conditional statement
From: Gonglei arei.gong...@huawei.com
Yoda conditions lack readability, and QEMU has a
strict compiler configuration for checking a common
mistake like if (dev = NULL). Make it a written rule.
Signed-off-by: Gonglei arei.gong...@huawei.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
CODING_STYLE | 14 ++
1 file changed, 14 insertions(+)
diff --git a/CODING_STYLE b/CODING_STYLE
index 4280945..d46cfa5 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -91,3 +91,17 @@ Mixed declarations (interleaving statements and declarations
within blocks)
are not allowed; declarations should be at the beginning of blocks. In other
words, the code should not generate warnings if using GCC's
-Wdeclaration-after-statement option.
+
+6. Conditional statements
+
+When comparing a variable for (in)equality with a constant, list the
+constant on the right, as in:
+
+if (a == 1) {
+/* Reads like: If a equals 1 */
+do_something();
+}
+
+Rationale: Yoda conditions (as in 'if (1 == a)') are awkward to read.
+Besides, good compilers already warn users when '==' is mis-typed as '=',
+even when the constant is on the right.
--
1.7.12.4
[Qemu-devel] [PATCH v4 0/8] don't use Yoda conditions
From: Gonglei arei.gong...@huawei.com $WHATEVER: don't use 'Yoda conditions' 'Yoda conditions' are not part of idiomatic QEMU coding style, so rewrite them in the more usual order. v4: - trivial typo for patch 1/8 suggested by Eric, thanks. v3: - rewrite CODINT_STYLE file suggested by Eric, thanks. - rename the patch serials. - imitate nearby code about using '!value' or 'value == NULL' at every patch suggested by Markus. v2: - add more specific commit messages suggested by PMM, thanks. - introduce section of conditional statement to CODING_STYLE. Gonglei (8): CODING_STYLE: Section about conditional statement usb: don't use 'Yoda conditions' audio: don't use 'Yoda conditions' isa-bus: don't use 'Yoda conditions' don't use 'Yoda conditions' spice: don't use 'Yoda conditions' vl: don't use 'Yoda conditions' vmxnet3: don't use 'Yoda conditions' CODING_STYLE | 14 ++ hw/audio/gus.c | 2 +- hw/audio/hda-codec.c | 3 ++- hw/audio/sb16.c | 6 +++--- hw/isa/isa-bus.c | 2 +- hw/net/vmxnet3.c | 16 hw/usb/dev-audio.c | 2 +- hw/usb/dev-mtp.c | 4 ++-- hw/usb/hcd-ehci.c| 2 +- qdev-monitor.c | 2 +- qemu-char.c | 2 +- ui/spice-core.c | 4 ++-- util/qemu-sockets.c | 2 +- vl.c | 5 +++-- 14 files changed, 41 insertions(+), 25 deletions(-) -- 1.7.12.4
[Qemu-devel] [PATCH v4 4/8] isa-bus: don't use 'Yoda conditions'
From: Gonglei arei.gong...@huawei.com
imitate nearby code about using '!value' or 'value == NULL'
Signed-off-by: Gonglei arei.gong...@huawei.com
---
hw/isa/isa-bus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index b28981b..cc85e53 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -50,7 +50,7 @@ ISABus *isa_bus_new(DeviceState *dev, MemoryRegion
*address_space_io)
fprintf(stderr, Can't create a second ISA bus\n);
return NULL;
}
-if (NULL == dev) {
+if (!dev) {
dev = qdev_create(NULL, isabus-bridge);
qdev_init_nofail(dev);
}
--
1.7.12.4
[Qemu-devel] [PATCH v4 2/8] usb: don't use 'Yoda conditions'
From: Gonglei arei.gong...@huawei.com
imitate nearby code about using '!value' or 'value == NULL'
Signed-off-by: Gonglei arei.gong...@huawei.com
---
hw/usb/dev-audio.c | 2 +-
hw/usb/dev-mtp.c | 4 ++--
hw/usb/hcd-ehci.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c
index bfebfe9..7b9957b 100644
--- a/hw/usb/dev-audio.c
+++ b/hw/usb/dev-audio.c
@@ -371,7 +371,7 @@ static void output_callback(void *opaque, int avail)
return;
}
data = streambuf_get(s-out.buf);
-if (NULL == data) {
+if (!data) {
return;
}
AUD_write(s-out.voice, data, USBAUDIO_PACKET_SIZE);
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 384d4a5..0820046 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -832,7 +832,7 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
return;
}
data_in = usb_mtp_get_object(s, c, o);
-if (NULL == data_in) {
+if (data_in == NULL) {
usb_mtp_queue_result(s, RES_GENERAL_ERROR,
c-trans, 0, 0, 0);
return;
@@ -851,7 +851,7 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
return;
}
data_in = usb_mtp_get_partial_object(s, c, o);
-if (NULL == data_in) {
+if (data_in == NULL) {
usb_mtp_queue_result(s, RES_GENERAL_ERROR,
c-trans, 0, 0, 0);
return;
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index a00a93c..448e007 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1596,7 +1596,7 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int
async)
entry = ehci_get_fetch_addr(ehci, async);
q = ehci_find_queue_by_qh(ehci, entry, async);
-if (NULL == q) {
+if (q == NULL) {
q = ehci_alloc_queue(ehci, entry, async);
}
--
1.7.12.4
Re: [Qemu-devel] [PATCH v2 2/7] qemu-img: Add progress output for amend
The Saturday 02 Aug 2014 à 01:49:16 (+0200), Max Reitz wrote :
Now that bdrv_amend_options() supports a status callback, use it to
display a progress report.
Signed-off-by: Max Reitz mre...@redhat.com
---
qemu-img-cmds.hx | 4 ++--
qemu-img.c | 25 ++---
qemu-img.texi| 2 +-
3 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 55aec6b..d83f3d0 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -70,8 +70,8 @@ STEXI
ETEXI
DEF(amend, img_amend,
-amend [-q] [-f fmt] [-t cache] -o options filename)
+amend [-p] [-q] [-f fmt] [-t cache] -o options filename)
STEXI
-@item amend [-q] [-f @var{fmt}] [-t @var{cache}] -o @var{options}
@var{filename}
+@item amend [-p] [-q] [-f @var{fmt}] [-t @var{cache}] -o @var{options}
@var{filename}
@end table
ETEXI
diff --git a/qemu-img.c b/qemu-img.c
index 90d6b79..4b6406b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2732,6 +2732,12 @@ out:
return 0;
}
+static void amend_status_cb(BlockDriverState *bs,
+int64_t offset, int64_t total_work_size)
+{
+qemu_progress_print(100.f * offset / total_work_size, 0);
+}
+
static int img_amend(int argc, char **argv)
{
int c, ret = 0;
@@ -2740,12 +2746,12 @@ static int img_amend(int argc, char **argv)
QemuOpts *opts = NULL;
const char *fmt = NULL, *filename, *cache;
int flags;
-bool quiet = false;
+bool quiet = false, progress = false;
BlockDriverState *bs = NULL;
cache = BDRV_DEFAULT_CACHE;
for (;;) {
-c = getopt(argc, argv, ho:f:t:q);
+c = getopt(argc, argv, ho:f:t:pq);
if (c == -1) {
break;
}
@@ -2775,6 +2781,9 @@ static int img_amend(int argc, char **argv)
case 't':
cache = optarg;
break;
+case 'p':
+progress = true;
+break;
case 'q':
quiet = true;
break;
@@ -2785,6 +2794,11 @@ static int img_amend(int argc, char **argv)
error_exit(Must specify options (-o));
}
+if (quiet) {
+progress = false;
+}
+qemu_progress_init(progress, 1.0);
+
filename = (optind == argc - 1) ? argv[argc - 1] : NULL;
if (fmt has_help_option(options)) {
/* If a format is explicitly specified (and possibly no filename is
@@ -2827,13 +2841,18 @@ static int img_amend(int argc, char **argv)
goto out;
}
-ret = bdrv_amend_options(bs, opts, NULL);
+/* In case the driver does not call amend_status_cb() */
+qemu_progress_print(0.f, 0);
+ret = bdrv_amend_options(bs, opts, amend_status_cb);
+qemu_progress_print(100.f, 0);
if (ret 0) {
error_report(Error while amending options: %s, strerror(-ret));
goto out;
}
out:
+qemu_progress_end();
+
if (bs) {
bdrv_unref(bs);
}
diff --git a/qemu-img.texi b/qemu-img.texi
index cb68948..2c66603 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -397,7 +397,7 @@ After using this command to grow a disk image, you must
use file system and
partitioning tools inside the VM to actually begin using the new space on the
device.
-@item amend [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{filename}
+@item amend [-p] [-f @var{fmt}] [-t @var{cache}] -o @var{options}
@var{filename}
Amends the image format specific @var{options} for the image file
@var{filename}. Not all file formats support this operation.
--
2.0.3
Reviewed-by: Benoit Canet ben...@irqsave.net
[Qemu-devel] [PULL] virtio-rng: fix memleak, use error_setg
Hi, This patchset fixes a memleak in virtio-rng init and moves from error_set to error_setg. Please pull. The following changes since commit c79805802ba0463713c253307d99ebef56436b8c: Open 2.2 development tree (2014-08-01 18:30:08 +0100) are available in the git repository at: git://git.kernel.org/pub/scm/virt/qemu/amit/virtio-rng.git for-2.2 for you to fetch changes up to c617dd3b7e8e82511060b8f7a9c51e46c5c1e87a: virtio-rng: replace error_set calls with error_setg (2014-08-04 14:50:11 +0530) John Snow (2): virtio-rng: Move error-checking forward to prevent memory leak virtio-rng: replace error_set calls with error_setg hw/virtio/virtio-rng.c | 25 - 1 file changed, 12 insertions(+), 13 deletions(-) Amit
Re: [Qemu-devel] [PATCH V4 2/5] runner: Tool for fuzz tests execution
On Fri, Aug 1, 2014 at 9:46 AM, Stefan Hajnoczi stefa...@gmail.com wrote:
On Mon, Jul 21, 2014 at 02:18:09PM +0400, Maria Kustova wrote:
+def execute(self, input_commands=None, fuzz_config=None):
+ Execute a test.
+
+The method creates backing and test images, runs test app and
analyzes
+its exit status. If the application was killed by a signal, the test
+is marked as failed.
+
+if input_commands is None:
+commands = self.commands
+else:
+commands = input_commands
+os.chdir(self.current_dir)
+backing_file_name, backing_file_fmt = self._create_backing_file()
+img_size = image_generator.create_image('test_image',
+backing_file_name,
+backing_file_fmt,
+fuzz_config)
+for item in commands:
+start = random.randint(0, img_size)
+end = random.randint(start, img_size)
+current_cmd = list(self.__dict__[item[0].replace('-', '_')])
This special case for self.qemu_img and self.qemu_io is not obvious to
me when reading the code. It would be clearer to use the same
$qemu_img/$qemu_io substitution mechanism as for $test_img/$off/$len
below.
Separate processing of test programs is due to the impossibility to
laconically replace a string with an unpacked list of strings.
But I agree that using of __dict__ is not safe and have to be redesigned.
BR, M.
Re: [Qemu-devel] [PATCH v2 4/7] block/qcow2: Implement status CB for amend
The Saturday 02 Aug 2014 à 01:49:18 (+0200), Max Reitz wrote :
The only really time-consuming operation potentially performed by
qcow2_amend_options() is zero cluster expansion when downgrading qcow2
images from compat=1.1 to compat=0.10, so report status of that
operation and that operation only through the status CB.
For this, approximate the progress as the number of L1 entries visited
during the operation.
Signed-off-by: Max Reitz mre...@redhat.com
---
block/qcow2-cluster.c | 37 +
block/qcow2.c | 7 ---
block/qcow2.h | 3 ++-
3 files changed, 39 insertions(+), 8 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 4208dc0..2607715 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1548,10 +1548,17 @@ fail:
* zero expansion (i.e., has been filled with zeroes and is referenced from
an
* L2 table). nb_clusters contains the total cluster count of the image file,
* i.e., the number of bits in expanded_clusters.
+ *
+ * l1_entries and *visited_l1_entries are used to keep track of progress for
+ * status_cb(). l1_entries contains the total number of L1 entries and
+ * *visited_l1_entries counts all visited L1 entries.
*/
static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t
*l1_table,
int l1_size, uint8_t
**expanded_clusters,
- uint64_t *nb_clusters)
+ uint64_t *nb_clusters,
+ int64_t *visited_l1_entries,
+ int64_t l1_entries,
+ BlockDriverAmendStatusCB *status_cb)
{
BDRVQcowState *s = bs-opaque;
bool is_active_l1 = (l1_table == s-l1_table);
@@ -1571,6 +1578,10 @@ static int expand_zero_clusters_in_l1(BlockDriverState
*bs, uint64_t *l1_table,
if (!l2_offset) {
/* unallocated */
+(*visited_l1_entries)++;
+if (status_cb) {
+status_cb(bs, *visited_l1_entries, l1_entries);
+}
continue;
}
@@ -1703,6 +1714,11 @@ static int expand_zero_clusters_in_l1(BlockDriverState
*bs, uint64_t *l1_table,
}
}
}
+
+(*visited_l1_entries)++;
+if (status_cb) {
+status_cb(bs, *visited_l1_entries, l1_entries);
+}
}
ret = 0;
@@ -1729,21 +1745,32 @@ fail:
* allocation for pre-allocated ones). This is important for downgrading to a
* qcow2 version which doesn't yet support metadata zero clusters.
*/
-int qcow2_expand_zero_clusters(BlockDriverState *bs)
+int qcow2_expand_zero_clusters(BlockDriverState *bs,
+ BlockDriverAmendStatusCB *status_cb)
{
BDRVQcowState *s = bs-opaque;
uint64_t *l1_table = NULL;
uint64_t nb_clusters;
+int64_t l1_entries = 0, visited_l1_entries = 0;
uint8_t *expanded_clusters;
int ret;
int i, j;
+if (status_cb) {
+l1_entries = s-l1_size;
+for (i = 0; i s-nb_snapshots; i++) {
+l1_entries += s-snapshots[i].l1_size;
+}
+}
+
nb_clusters = size_to_clusters(s, bs-file-total_sectors *
BDRV_SECTOR_SIZE);
expanded_clusters = g_malloc0((nb_clusters + 7) / 8);
ret = expand_zero_clusters_in_l1(bs, s-l1_table, s-l1_size,
- expanded_clusters, nb_clusters);
+ expanded_clusters, nb_clusters,
+ visited_l1_entries, l1_entries,
+ status_cb);
if (ret 0) {
goto fail;
}
@@ -1777,7 +1804,9 @@ int qcow2_expand_zero_clusters(BlockDriverState *bs)
}
ret = expand_zero_clusters_in_l1(bs, l1_table,
s-snapshots[i].l1_size,
- expanded_clusters, nb_clusters);
+ expanded_clusters, nb_clusters,
+ visited_l1_entries, l1_entries,
+ status_cb);
if (ret 0) {
goto fail;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index a3a7efc..6e8c8ab 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2147,7 +2147,8 @@ static int qcow2_load_vmstate(BlockDriverState *bs,
uint8_t *buf,
* Downgrades an image's version. To achieve this, any incompatible features
* have to be removed.
*/
-static int qcow2_downgrade(BlockDriverState *bs, int target_version)
+static int qcow2_downgrade(BlockDriverState *bs, int target_version,
+ BlockDriverAmendStatusCB *status_cb)
{
BDRVQcowState *s = bs-opaque;
int current_version =
Re: [Qemu-devel] [PATCH] configure: replace \n with space in optarg
On Mon, Aug 04, 2014 at 09:50:20AM +0100, Peter Maydell wrote: On 4 August 2014 08:12, Hu Tao hu...@cn.fujitsu.com wrote: When optarg happens to contain \n like: ../configure --target-list='i386-softmmu x86_64-softmmu' make will fail with message: config-host.mak:45: *** missing separator. Stop. Why would you put newlines in the option string? This is likely to break for many configure arguments, not just this one... It is not intended but once when I did copypaste the string it brought a \n. I'm a bit dubious about taking this change unless you can point to existing practice (eg do autoconf configure scripts clean up whitespace like this?) I doubt there is. thanks -- PMM
Re: [Qemu-devel] [PATCH v2 5/7] block/qcow2: Make get_refcount() global
The Saturday 02 Aug 2014 à 01:49:19 (+0200), Max Reitz wrote :
Reading the refcount of a cluster is an operation which can be useful in
all of the qcow2 code, so make that function globally available.
While touching this function, amend the comment describing the addend
parameter: It is (no longer, if it ever was) necessary to have it set to
-1 or 1; any value is fine.
Signed-off-by: Max Reitz mre...@redhat.com
---
block/qcow2-refcount.c | 26 +-
block/qcow2.h | 2 ++
2 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index cc6cf74..1c2ab8c 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -87,7 +87,7 @@ static int load_refcount_block(BlockDriverState *bs,
* return value is the refcount of the cluster, negative values are -errno
* and indicate an error.
*/
-static int get_refcount(BlockDriverState *bs, int64_t cluster_index)
+int qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index)
{
BDRVQcowState *s = bs-opaque;
uint64_t refcount_table_index, block_index;
@@ -605,8 +605,7 @@ fail:
}
/*
- * Increases or decreases the refcount of a given cluster by one.
- * addend must be 1 or -1.
+ * Increases or decreases the refcount of a given cluster.
*
* If the return value is non-negative, it is the new refcount of the
cluster.
* If it is negative, it is -errno and indicates an error.
@@ -625,7 +624,7 @@ int qcow2_update_cluster_refcount(BlockDriverState *bs,
return ret;
}
-return get_refcount(bs, cluster_index);
+return qcow2_get_refcount(bs, cluster_index);
}
@@ -646,7 +645,7 @@ static int64_t alloc_clusters_noref(BlockDriverState *bs,
uint64_t size)
retry:
for(i = 0; i nb_clusters; i++) {
uint64_t next_cluster_index = s-free_cluster_index++;
-refcount = get_refcount(bs, next_cluster_index);
+refcount = qcow2_get_refcount(bs, next_cluster_index);
if (refcount 0) {
return refcount;
@@ -710,7 +709,7 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs,
uint64_t offset,
/* Check how many clusters there are free */
cluster_index = offset s-cluster_bits;
for(i = 0; i nb_clusters; i++) {
-refcount = get_refcount(bs, cluster_index++);
+refcount = qcow2_get_refcount(bs, cluster_index++);
if (refcount 0) {
return refcount;
@@ -927,7 +926,7 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
cluster_index, addend,
QCOW2_DISCARD_SNAPSHOT);
} else {
-refcount = get_refcount(bs, cluster_index);
+refcount = qcow2_get_refcount(bs, cluster_index);
}
if (refcount 0) {
@@ -967,7 +966,7 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
refcount = qcow2_update_cluster_refcount(bs, l2_offset
s-cluster_bits, addend, QCOW2_DISCARD_SNAPSHOT);
} else {
-refcount = get_refcount(bs, l2_offset s-cluster_bits);
+refcount = qcow2_get_refcount(bs, l2_offset
s-cluster_bits);
}
if (refcount 0) {
ret = refcount;
@@ -1243,8 +1242,8 @@ fail:
* Checks the OFLAG_COPIED flag for all L1 and L2 entries.
*
* This function does not print an error message nor does it increment
- * check_errors if get_refcount fails (this is because such an error will
have
- * been already detected and sufficiently signaled by the calling function
+ * check_errors if qcow2_get_refcount fails (this is because such an error
will
+ * have been already detected and sufficiently signaled by the calling
function
* (qcow2_check_refcounts) by the time this function is called).
*/
static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,
@@ -1265,7 +1264,7 @@ static int check_oflag_copied(BlockDriverState *bs,
BdrvCheckResult *res,
continue;
}
-refcount = get_refcount(bs, l2_offset s-cluster_bits);
+refcount = qcow2_get_refcount(bs, l2_offset s-cluster_bits);
if (refcount 0) {
/* don't print message nor increment check_errors */
continue;
@@ -1307,7 +1306,8 @@ static int check_oflag_copied(BlockDriverState *bs,
BdrvCheckResult *res,
if ((cluster_type == QCOW2_CLUSTER_NORMAL) ||
((cluster_type == QCOW2_CLUSTER_ZERO) (data_offset !=
0))) {
-refcount = get_refcount(bs, data_offset s-cluster_bits);
+refcount = qcow2_get_refcount(bs,
+ data_offset
s-cluster_bits);
Re: [Qemu-devel] [PATCH v2 6/7] block/qcow2: Simplify shared L2 handling in amend
The Saturday 02 Aug 2014 à 01:49:20 (+0200), Max Reitz wrote :
Currently, we have a bitmap for keeping track of which clusters have
been created during the zero cluster expansion process. This was
necessary because we need to properly increase the refcount for shared
L2 tables.
However, now we can simply take the L2 refcount and use it for the
cluster allocated for expansion. This will be the correct refcount and
therefore we don't have to remember that cluster having been allocated
any more.
Signed-off-by: Max Reitz mre...@redhat.com
---
block/qcow2-cluster.c | 90
---
1 file changed, 28 insertions(+), 62 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 2607715..7e65c13 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1543,20 +1543,12 @@ fail:
* Expands all zero clusters in a specific L1 table (or deallocates them, for
* non-backed non-pre-allocated zero clusters).
*
- * expanded_clusters is a bitmap where every bit corresponds to one cluster
in
- * the image file; a bit gets set if the corresponding cluster has been used
for
- * zero expansion (i.e., has been filled with zeroes and is referenced from
an
- * L2 table). nb_clusters contains the total cluster count of the image file,
- * i.e., the number of bits in expanded_clusters.
- *
* l1_entries and *visited_l1_entries are used to keep track of progress for
* status_cb(). l1_entries contains the total number of L1 entries and
* *visited_l1_entries counts all visited L1 entries.
*/
static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t
*l1_table,
- int l1_size, uint8_t
**expanded_clusters,
- uint64_t *nb_clusters,
- int64_t *visited_l1_entries,
+ int l1_size, int64_t
*visited_l1_entries,
int64_t l1_entries,
BlockDriverAmendStatusCB *status_cb)
{
@@ -1575,6 +1567,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState
*bs, uint64_t *l1_table,
for (i = 0; i l1_size; i++) {
uint64_t l2_offset = l1_table[i] L1E_OFFSET_MASK;
bool l2_dirty = false;
+int l2_refcount;
if (!l2_offset) {
/* unallocated */
@@ -1598,33 +1591,19 @@ static int
expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
goto fail;
}
+l2_refcount = qcow2_get_refcount(bs, l2_offset s-cluster_bits);
+if (l2_refcount 0) {
+ret = l2_refcount;
+goto fail;
+}
+
for (j = 0; j s-l2_size; j++) {
uint64_t l2_entry = be64_to_cpu(l2_table[j]);
-int64_t offset = l2_entry L2E_OFFSET_MASK, cluster_index;
+int64_t offset = l2_entry L2E_OFFSET_MASK;
int cluster_type = qcow2_get_cluster_type(l2_entry);
bool preallocated = offset != 0;
-if (cluster_type == QCOW2_CLUSTER_NORMAL) {
-cluster_index = offset s-cluster_bits;
-assert((cluster_index = 0) (cluster_index
*nb_clusters));
-if ((*expanded_clusters)[cluster_index / 8]
-(1 (cluster_index % 8))) {
-/* Probably a shared L2 table; this cluster was a zero
- * cluster which has been expanded, its refcount
- * therefore most likely requires an update. */
-ret = qcow2_update_cluster_refcount(bs, cluster_index, 1,
-QCOW2_DISCARD_NEVER);
-if (ret 0) {
-goto fail;
-}
-/* Since we just increased the refcount, the COPIED flag
may
- * no longer be set. */
-l2_table[j] = cpu_to_be64(l2_entry ~QCOW_OFLAG_COPIED);
-l2_dirty = true;
-}
-continue;
-}
-else if (qcow2_get_cluster_type(l2_entry) != QCOW2_CLUSTER_ZERO)
{
+if (cluster_type != QCOW2_CLUSTER_ZERO) {
continue;
}
@@ -1642,6 +1621,19 @@ static int expand_zero_clusters_in_l1(BlockDriverState
*bs, uint64_t *l1_table,
ret = offset;
goto fail;
}
+
+if (l2_refcount 1) {
+/* For shared L2 tables, set the refcount accordingly
(it is
+ * already 1 and needs to be l2_refcount) */
+ret = qcow2_update_cluster_refcount(bs,
+offset s-cluster_bits, l2_refcount - 1,
+
[Qemu-devel] [PULL] migration: static checker: handle 'unused' fields, whitelist update
Hello, This patchset updates the vmstate static checker to handle fields that got marked 'unused' in qemu versions. Also update the whitelist based on a few false-positives. The following changes since commit 35858955e6c6f9ef41c199d15457c13426ac6434: Merge remote-tracking branch 'remotes/afaerber/tags/qom-devices-for-2.1' into staging (2014-07-21 18:06:12 +0100) are available in the git repository at: git://git.kernel.org/pub/scm/virt/qemu/amit/migration.git for-2.2 for you to fetch changes up to 32ce1b4817e8341f55906dc003cc09ae22502ea7: checker: ignore fields marked unused (2014-08-04 15:02:37 +0530) Amit Shah (2): vmstate static checker: whitelist additions checker: ignore fields marked unused scripts/vmstate-static-checker.py | 70 +- 1 file changed, 65 insertions(+), 5 deletions(-) Amit
Re: [Qemu-devel] [PULL] migration: static checker: handle 'unused' fields, whitelist update
On (Mon) 04 Aug 2014 [15:06:05], Amit Shah wrote: Hello, This patchset updates the vmstate static checker to handle fields that got marked 'unused' in qemu versions. Also update the whitelist based on a few false-positives. The following changes since commit 35858955e6c6f9ef41c199d15457c13426ac6434: Merge remote-tracking branch 'remotes/afaerber/tags/qom-devices-for-2.1' into staging (2014-07-21 18:06:12 +0100) Oops, didn't rebase, but doesn't matter as the static checker didn't see any updates in the meantime. Amit
Re: [Qemu-devel] [ANNOUNCE] QEMU 2.1.0 is now available
Am 02.08.2014 um 00:33 hat Peter Maydell geschrieben: On 1 August 2014 17:41, Michael Roth mdr...@linux.vnet.ibm.com wrote: On behalf of the QEMU Team, I'd like to announce the availability of the QEMU 2.1.0 release. This release contains 2200+ commits from 180 authors. Thank you to everyone involved! Yep, thanks to everybody who helped get this one out of the door; trunk is now re-opened for 2.2 development. If there's anything you think we should maybe try to do better or differently next time round, now might be a good time to suggest it. Personally I think it went reasonably well and I was happy with the length of hardfreeze time. I agree, that seems to have been a good length for the freeze. It happened probably for the first time that in the end an RC was approaching and I simply didn't have any new patches to submit a pull request for. It feels a lot better when you don't have masses of last-minute fixes, but things actually stabilise a bit. We should remember to ask for updates of the message translations at rc0/rc1 time rather than having them appear very late in the cycle, perhaps. That's a good point. Should we include it in a Planning/2.2 wiki page so we won't forget, or is there a better place for it? Kevin
Re: [Qemu-devel] [PATCH 26/28] ahci: Add test_hba_spec to ahci-test.
On Fri, Aug 01, 2014 at 07:27:57PM -0400, John Snow wrote:
On 07/31/2014 10:01 AM, Stefan Hajnoczi wrote:
On Mon, Jul 07, 2014 at 02:18:07PM -0400, John Snow wrote:
+/*** IO macros for the AHCI memory registers. ***/
+#define void_incr(vptr, OFST) ((void *)((char *)(vptr) + (OFST)))
I'm pretty sure QEMU takes advantage of GCC's void pointer arithmetic
extension:
https://gcc.gnu.org/onlinedocs/gcc-4.9.1/gcc/Pointer-Arith.html#Pointer-Arith
In other words, vptr + OFST works and you don't need a macro.
+#define ahci_set(regno, mask) ahci_wreg((regno), ahci_rreg(regno) | (mask))
+#define ahci_clr(regno, mask) ahci_wreg((regno), ahci_rreg(regno)
~(mask))
Unused. Please move to the patch that actually uses them.
+#define px_set(port, reg, mask) px_wreg((port), (reg), \
+ px_rreg((port), (reg)) | (mask));
+#define px_clr(port, reg, mask) px_wreg((port), (reg), \
+ px_rreg((port), (reg)) ~(mask));
Unused. Please move to the patch that actually uses them.
+/* We need to know the size of the region,
+ * but qpci_iomap doesn't save it. Recalculate it. */
It seems like many tests will want to check the BAR size. Please add an
argument to qpci_iomap() so the caller gets the size.
+if (bitset(cap, AHCI_CAP_SAM)) {
+g_test_message(Supports AHCI-Only Mode: GHC_AE is Read-Only.);
+assert_bit_set(reg, AHCI_GHC_AE);
+} else {
+g_test_message(Supports AHCI/Legacy mix.);
+assert_bit_clear(reg, AHCI_GHC_AE);
+}
Let's just assert what QEMU implements.
+/* 12 -- 23: Reserved */
+g_test_message(Verifying HBA reserved area is empty.);
Debugging message that can be removed?
More elsewhere in this patch.
one last question -- is there a reasonable way to have assertions that allow
the test to shamble forward, still fail at the end, and still run subsequent
test cases?
A lot of the warnings I threw in here are actually errors, but it'd still be
useful to see the rest of the test run to completion as best as it can.
You can call g_test_set_nonfatal_assertions(), but it actually appears as if
(on my machine at least) that this is a nop, which is not super helpful.
It'd be nice if the checked in version of the test showed you the myriad
failings, instead of just one at a time until you hack in/out certain
assertions manually if you are interested in other areas.
I'm not aware of a glib API to do that. You can tell gtester which test
cases to invoke if you need to skip or only run certain cases. That
might be useful during development.
Stefan
pgpfuLEt9CyzL.pgp
Description: PGP signature
Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex command
Gonglei (Arei) arei.gong...@huawei.com writes:
Hi,
Subject: Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex command
On Fri, Jul 25, 2014 at 02:52:50PM +0800, arei.gong...@huawei.com wrote:
From: Gonglei arei.gong...@huawei.com
Adds set-bootindex id=xx,bootindex=xx,suffix=xx QMP command.
Example QMP command:
- { execute: set-bootindex, arguments: { id: ide0-0-1,
bootindex:
1, suffix: /disk@0}}
- { return: {} }
Signed-off-by: Gonglei arei.gong...@huawei.com
Signed-off-by: Chenliang chenlian...@huawei.com
---
qapi-schema.json | 16
qmp-commands.hx | 24
qmp.c| 17 +
3 files changed, 57 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json
index b11aad2..a9ef0be 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1704,6 +1704,22 @@
{ 'command': 'device_del', 'data': {'id': 'str'} }
##
+# @set-bootindex:
+#
+# set bootindex of a devcie
+#
+# @id: the name of the device
+# @bootindex: the bootindex of the device
+# @suffix: #optional a suffix of the device
+#
+# Returns: Nothing on success
+# If @id is not a valid device, DeviceNotFound
+#
+# Since: 2.2
+##
+{ 'command': 'set-bootindex', 'data': {'id': 'str', 'bootindex':
int', '*suffix':
'str'} }
+
+##
I wonder if we could simply use qom-set for that. How many devices
actually support having multiple bootindex entries with different
suffixes?
AFAICT, the floppy device support two bootindex with different suffixes.
Block device models commonly a single block device. By convention,
property drive is the backend, and property bootindex the bootindex,
if the device model supports that.
The device ID suffices to identify a block device for such device
models.
The floppy device model is an exception. It folds multiple real-world
objects into one: the controller and the actual drives. Have a look at
-device isa-fdc,help:
isa-fdc.iobase=uint32
isa-fdc.irq=uint32
isa-fdc.dma=uint32
isa-fdc.driveA=drive
isa-fdc.driveB=drive
isa-fdc.bootindexA=int32
isa-fdc.bootindexB=int32
isa-fdc.check_media_rate=on/off
The properties ending with 'A' or 'B' apply to the first and the second
drive, the others to the controller.
Unfortunately, this means the device ID by itself doesn't identify the
floppy device.
Yes.
Arguably, this is lousy modeling --- we really should model separate
real-world objects as separate objects. But making floppies pretty (or
even sane) isn't anyone's priority nowadays.
Hmm... Agreed.
Since qom-set works on *properties*, I can't see why it couldn't be used
for changing bootindexes. There is no ambiguity even with floppy.
Sorry?
You either set bootindexA or bootindexB. No need to fuzz around with
suffixes. Or am I missing something?
Your mean that we just need to think about change one bootindex? Either
set bootindexA or bootindexB, do not need pass suffix for identifying?
Eduardo suggested to think about using qom-set to update the bootindex.
Could look like
{ execute: qom-set,
arguments: { path: /machine/unattached/device[15],
property: bootindexA, value: 1 } }
Demonstrates an existing, well-defined way to identify the bootindex to
change. Do we really want to invent another one, based on suffix?
The value of path is the QOM path. I can't remember offhand how to go
from qdev ID to QOM path. Onboard devices like isa-fdc don't have one
anyway.
I also don't remember whether there's a nicer QOM path than this one.
Re: [Qemu-devel] [PATCH 00/14] dataplane: optimization and multi virtqueue support
On Wed, Jul 30, 2014 at 07:39:33PM +0800, Ming Lei wrote: These patches bring up below 4 changes: - introduce selective coroutine bypass mechanism for improving performance of virtio-blk dataplane with raw format image - introduce object allocation pool and apply it to virtio-blk dataplane for improving its performance - linux-aio changes: fixing for cases of -EAGAIN and partial completion, increase max events to 256, and remove one unuseful fields in 'struct qemu_laiocb' - support multi virtqueue for virtio-blk dataplane Please split this series into separate series. Patch 1-5 - block layer coroutine bypass These patches are hacky and not correct yet (e.g. you cannot enable bypass on a per-AioContext basis since multiple devices can share an IOThread's AioContext and some of them might not be raw, you are invoking acb's cb() directly instead of via a BH so there may be reentrancy). It would be great if cleaning up the block.c aio-co emulation layers could improve performance. pgpXLPONIUB8C.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH 07/15] dataplane: use object pool to speed up allocation for virtio blk request
On Fri, Aug 01, 2014 at 03:42:05PM +0800, Ming Lei wrote: On Thu, Jul 31, 2014 at 5:18 PM, Paolo Bonzini pbonz...@redhat.com wrote: Il 31/07/2014 05:22, Ming Lei ha scritto: The problem is that g_slice here is not using the slab-style allocator because the object is larger than roughly 500 bytes. One solution would be to make virtqueue_pop/vring_pop allocate a VirtQueueElement of the right size (and virtqueue_push/vring_push free it), as mentioned in the review of patch 8. Unluckily both iovec and addr array can't be fitted into 500 bytes, :-( Not mention all users of VirtQueueElement need to be changed too, I hate to make that work involved in this patchset, :-) Well, the point of dataplane was not just to get maximum iops. It was also to provide guidance in the work necessary to improve the code and get maximum iops without special-casing everything. This can be a lot of work indeed. However, I now remembered that VirtQueueElement is a mess because it's serialized directly into the migration state. :( So you basically cannot change it without mucking with migration. Please leave out patch 8 for now. save_device code serializes elem in this way: qemu_put_buffer(f, (unsigned char *)req-elem, sizeof(VirtQueueElement)); so I am wondering why this patch may break migration. Because you change the on-wire format and break migration from 2.1 to 2.2. Sorry, I wasn't clear enough. That is really a mess, but in future we still may convert VirtQueueElement into a smart one, and keep the original structure only for save/load, but a conversion between the two structures is required in save/load. This is a good idea. The VirtQueueElement structure isn't complicated, it's just big. Just use the current layout as the serialization format. Stefan pgpo19Y2Sg6dO.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH 10/15] linux-aio: increase max event to 256
On Thu, Jul 31, 2014 at 01:20:22AM +0800, Ming Lei wrote: On Wed, Jul 30, 2014 at 10:00 PM, Paolo Bonzini pbonz...@redhat.com wrote: Il 30/07/2014 13:39, Ming Lei ha scritto: This patch increases max event to 256 for the comming virtio-blk multi virtqueue support. Signed-off-by: Ming Lei ming@canonical.com --- block/linux-aio.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) What makes the new magic number less magic than the old one? Just for supporting the coming multi virtqueue, otherwise it is easy to trigger EAGAIN. Or do you have better idea to figure out a non-magic number? The virtio-blk device knows how many requests can be in-flight at a time. laio_init() could take a size parameter. The messy thing is that the block layer is between the virtio-blk device and Linux AIO, so we'd need to pass that information down. Stefan pgpVDlT80rRcr.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH RFC 0/3] dataplane: dataplane: more graceful error handling
On Fri, 25 Jul 2014 14:10:45 +0200 Cornelia Huck cornelia.h...@de.ibm.com wrote: Currently, qemu will take a hard exit if it fails to set up guest or host notifiers, giving no real clue as to what went wrong (e.g., when out of file descriptors). This patchset tries to make this more manageable: Both by improving the error message and by gracefully falling back to non-dataplane in case of errors. Patches are also available on git://github.com/cohuck/qemu dataplane-graceful-fail Thoughts? Cornelia Huck (3): dataplane: print why starting failed dataplane: fail notifier setting gracefully dataplane: stop trying on notifier error hw/block/dataplane/virtio-blk.c | 39 +-- 1 file changed, 29 insertions(+), 10 deletions(-) Ping?
Re: [Qemu-devel] [PATCH v2 0/5] AArch64 TLB performance improvements
Peter Maydell writes: On 1 August 2014 17:06, Peter Maydell peter.mayd...@linaro.org wrote: I'm taking the first two of these into target-arm.next because they're obvious standalone bugfixes. I need to think about the last three a bit more: I dislike just dropping the ARMv5 CPUs from qemu-system-aarch64, it's kind of arbitrary. So: * there's clearly a big perf win to be had here * this patchset gives us that for 4K pages on AArch64 * but it doesn't help for 4K pages on AArch32 (really common) Well for the AArch32 profile if you ran under qemu-system-aarch64 you would be OK surely? * and it's not going to be good for 64K pages on AArch64 either (which I suspect will not be a rare choice) Does the kernel already use 64k pages for it's code? So I think it would be good if we investigated the degree of difficulty in improving QEMU's TLB code so it isn't just one TLB entry size with larger pages a bolt-on which we hope people don't actually use first, before we just disable all the v5 CPUs. Given there is likely to be a growth of multiple-page size guests we probably do want to look at cleaning up the TLB code to handle these cases gracefully. Another option we could look at is keeping track of cross-page TB links and then invalidating them if we need to. We might want to do that based on heuristics so avoid excessive cleaning up. However you would expect for example the kernel to sit in it's own set of bigger pages which never get invalidated where we could happily chain more TBs together. thanks -- PMM -- Alex Bennée
Re: [Qemu-devel] [PATCH v2 0/5] AArch64 TLB performance improvements
Paolo Bonzini writes: Il 30/07/2014 17:20, Alex Bennée ha scritto: Hi, snip The most important thing is I've measured a 25-30% improvement in kernel and android boot time. snip Hi Alex, have you seen this patch? Perhaps you're interested in reviving it. http://article.gmane.org/gmane.comp.emulators.qemu/253864 I saw it when it first came out but I didn't quite follow what it was doing as I hadn't looked at the TLB code. I'll have another look and see what difference it can make. Paolo -- Alex Bennée
Re: [Qemu-devel] fpu/softfloat.c licensing
On 08/04/2014 06:48 PM, Peter Maydell wrote: On 4 August 2014 05:55, Alexey Kardashevskiy a...@ozlabs.ru wrote: Our lawyers refused to provide any public advise on this :-/ Is that it, end of story? :) Well, I 'd still like to get the license fixed to at least my personal satisfaction, but if your lawyers won't confirm whether what we propose will work for them then there is simply no possible way that we can guarantee to make them happy. No, I am not trying to make them happy here :) I am trying to understand what the community is going to do about it because I thought that something is still planned. -- Alexey
Re: [Qemu-devel] [PATCH 00/14] dataplane: optimization and multi virtqueue support
On Mon, Aug 4, 2014 at 6:16 PM, Stefan Hajnoczi stefa...@redhat.com wrote: On Wed, Jul 30, 2014 at 07:39:33PM +0800, Ming Lei wrote: These patches bring up below 4 changes: - introduce selective coroutine bypass mechanism for improving performance of virtio-blk dataplane with raw format image - introduce object allocation pool and apply it to virtio-blk dataplane for improving its performance - linux-aio changes: fixing for cases of -EAGAIN and partial completion, increase max events to 256, and remove one unuseful fields in 'struct qemu_laiocb' - support multi virtqueue for virtio-blk dataplane Please split this series into separate series. Patch 1-5 - block layer coroutine bypass These patches are hacky and not correct yet (e.g. you cannot enable bypass on a per-AioContext basis since multiple devices can share an IOThread's AioContext and some of them might not be raw, you are Good point, so the 'bypass' info should be moved to BlockDriverState. invoking acb's cb() directly instead of via a BH so there may be reentrancy). Yes, as Paolo pointed too, I will change to run acb's cb() via a BH in v1 patchset. It would be great if cleaning up the block.c aio-co emulation layers could improve performance. That is a good idea too. But from current profiling, coroutine is surely one of main causes for the performance regression. Thanks,
Re: [Qemu-devel] [PATCH v2 0/5] AArch64 TLB performance improvements
On 4 August 2014 11:23, Alex Bennée alex.ben...@linaro.org wrote: Peter Maydell writes: So: * there's clearly a big perf win to be had here * this patchset gives us that for 4K pages on AArch64 * but it doesn't help for 4K pages on AArch32 (really common) Well for the AArch32 profile if you ran under qemu-system-aarch64 you would be OK surely? Yes, but that's pretty non-obvious, and also it doesn't make much sense to the user to say these 32 bit CPUs should be run under qemu-system-aarch64 but these other ones should be under qemu-system-arm. * and it's not going to be good for 64K pages on AArch64 either (which I suspect will not be a rare choice) Does the kernel already use 64k pages for it's code? There's a config option, which will cause it to use 64K pages for everything including userspace. (There's also 16K pages but I forget if Linux has support for those.) I think the kernel can also use 64K pages in some cases even in a 4K page config, but I don't know the details. thanks -- PMM
[Qemu-devel] [PATCH v2 1/1] virtio-rng: add some trace events
Add some trace events to virtio-rng for easier debugging
Signed-off-by: Amit Shah amit.s...@redhat.com
---
v2:
- requested_size trace event now shows proper values
---
hw/virtio/virtio-rng.c | 6 ++
trace-events | 5 +
2 files changed, 11 insertions(+)
diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index 03fd04a..e85a979 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -16,6 +16,7 @@
#include hw/virtio/virtio-rng.h
#include sysemu/rng.h
#include qom/object_interfaces.h
+#include trace.h
static bool is_guest_ready(VirtIORNG *vrng)
{
@@ -24,6 +25,7 @@ static bool is_guest_ready(VirtIORNG *vrng)
(vdev-status VIRTIO_CONFIG_S_DRIVER_OK)) {
return true;
}
+trace_virtio_rng_guest_not_ready(vrng);
return false;
}
@@ -62,6 +64,7 @@ static void chr_read(void *opaque, const void *buf, size_t
size)
offset += len;
virtqueue_push(vrng-vq, elem, len);
+trace_virtio_rng_pushed(vrng, len);
}
virtio_notify(vdev, vrng-vq);
}
@@ -81,6 +84,9 @@ static void virtio_rng_process(VirtIORNG *vrng)
quota = MIN((uint64_t)vrng-quota_remaining, (uint64_t)UINT32_MAX);
}
size = get_request_size(vrng-vq, quota);
+
+trace_virtio_rng_request(vrng, size, quota);
+
size = MIN(vrng-quota_remaining, size);
if (size) {
rng_backend_request_entropy(vrng-rng, size, chr_read, vrng);
diff --git a/trace-events b/trace-events
index 11a17a8..99f39ac 100644
--- a/trace-events
+++ b/trace-events
@@ -41,6 +41,11 @@ virtio_irq(void *vq) vq %p
virtio_notify(void *vdev, void *vq) vdev %p vq %p
virtio_set_status(void *vdev, uint8_t val) vdev %p val %u
+# hw/virtio/virtio-rng.c
+virtio_rng_guest_not_ready(void *rng) rng %p: guest not ready
+virtio_rng_pushed(void *rng, size_t len) rng %p: %zd bytes pushed
+virtio_rng_request(void *rng, size_t size, unsigned quota) rng %p: %zd bytes
requested, %u bytes quota left
+
# hw/char/virtio-serial-bus.c
virtio_serial_send_control_event(unsigned int port, uint16_t event, uint16_t
value) port %u, event %u, value %u
virtio_serial_throttle_port(unsigned int port, bool throttle) port %u,
throttle %d
--
1.9.3
Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex command
Hi, Markus
Subject: Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex
command
On Fri, Jul 25, 2014 at 02:52:50PM +0800, arei.gong...@huawei.com
wrote:
From: Gonglei arei.gong...@huawei.com
Adds set-bootindex id=xx,bootindex=xx,suffix=xx QMP command.
Example QMP command:
- { execute: set-bootindex, arguments: { id: ide0-0-1,
bootindex:
1, suffix: /disk@0}}
- { return: {} }
Signed-off-by: Gonglei arei.gong...@huawei.com
Signed-off-by: Chenliang chenlian...@huawei.com
---
qapi-schema.json | 16
qmp-commands.hx | 24
qmp.c| 17 +
3 files changed, 57 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json
index b11aad2..a9ef0be 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1704,6 +1704,22 @@
{ 'command': 'device_del', 'data': {'id': 'str'} }
##
+# @set-bootindex:
+#
+# set bootindex of a devcie
+#
+# @id: the name of the device
+# @bootindex: the bootindex of the device
+# @suffix: #optional a suffix of the device
+#
+# Returns: Nothing on success
+# If @id is not a valid device, DeviceNotFound
+#
+# Since: 2.2
+##
+{ 'command': 'set-bootindex', 'data': {'id': 'str', 'bootindex':
int', '*suffix':
'str'} }
+
+##
I wonder if we could simply use qom-set for that. How many devices
actually support having multiple bootindex entries with different
suffixes?
AFAICT, the floppy device support two bootindex with different suffixes.
Block device models commonly a single block device. By convention,
property drive is the backend, and property bootindex the bootindex,
if the device model supports that.
The device ID suffices to identify a block device for such device
models.
The floppy device model is an exception. It folds multiple real-world
objects into one: the controller and the actual drives. Have a look at
-device isa-fdc,help:
isa-fdc.iobase=uint32
isa-fdc.irq=uint32
isa-fdc.dma=uint32
isa-fdc.driveA=drive
isa-fdc.driveB=drive
isa-fdc.bootindexA=int32
isa-fdc.bootindexB=int32
isa-fdc.check_media_rate=on/off
The properties ending with 'A' or 'B' apply to the first and the second
drive, the others to the controller.
Unfortunately, this means the device ID by itself doesn't identify the
floppy device.
Yes.
Arguably, this is lousy modeling --- we really should model separate
real-world objects as separate objects. But making floppies pretty (or
even sane) isn't anyone's priority nowadays.
Hmm... Agreed.
Since qom-set works on *properties*, I can't see why it couldn't be used
for changing bootindexes. There is no ambiguity even with floppy.
Sorry?
You either set bootindexA or bootindexB. No need to fuzz around with
suffixes. Or am I missing something?
Your mean that we just need to think about change one bootindex? Either
set bootindexA or bootindexB, do not need pass suffix for identifying?
Eduardo suggested to think about using qom-set to update the bootindex.
Could look like
{ execute: qom-set,
arguments: { path: /machine/unattached/device[15],
property: bootindexA, value: 1 } }
Demonstrates an existing, well-defined way to identify the bootindex to
change. Do we really want to invent another one, based on suffix?
The value of path is the QOM path. I can't remember offhand how to go
from qdev ID to QOM path. Onboard devices like isa-fdc don't have one
anyway.
I also don't remember whether there's a nicer QOM path than this one.
Thanks for your explaining. TBH I haven't used qom-set before, so I
don't know what your mean (like Eduardo, sorry again).
But now, I have a look at the implement of qom-set command, and
I find the command is just change a device's property value, do not have
any other logic. For my case, we really change value of the global
fw_boot_order list,
which the device's bootindex property worked for actually. Only in this way,
we can attach our original target.
Best regards,
-Gonglei
Re: [Qemu-devel] [PATCH 7/7] vmxnet3: a trivial code change for more idiomatic writing style
On Thu, Jul 31, 2014 at 08:29:00PM +0800, arei.gong...@huawei.com wrote: From: Gonglei arei.gong...@huawei.com Signed-off-by: Gonglei arei.gong...@huawei.com --- hw/net/vmxnet3.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) Reviewed-by: Stefan Hajnoczi stefa...@redhat.com pgpK5f516P9nj.pgp Description: PGP signature
Re: [Qemu-devel] fpu/softfloat.c licensing
Alexey Kardashevskiy a...@ozlabs.ru writes: On 08/04/2014 06:48 PM, Peter Maydell wrote: On 4 August 2014 05:55, Alexey Kardashevskiy a...@ozlabs.ru wrote: Our lawyers refused to provide any public advise on this :-/ Is that it, end of story? :) Well, I 'd still like to get the license fixed to at least my personal satisfaction, but if your lawyers won't confirm whether what we propose will work for them then there is simply no possible way that we can guarantee to make them happy. No, I am not trying to make them happy here :) I am trying to understand what the community is going to do about it because I thought that something is still planned. I'm afraid we need rip out the problematic patches. Here's Paolo's list: Fabrice Bellard fabr...@bellard.org 1d6bda356153c82e100680d9f2165e32c8fb1330 750afe93fd15fafc20b6c34d30f339547d15c2d1 Jocelyn Mayer 75d62a585629cdc1ae0d530189653cb1d8d9c53c Thiemo Seufer's parents (Stefan said he'd contact them) 5a6932d51d1b34b68b3f10fc5ac65598bece88c0 924b2c07cdfaba9ac408fc5fa77da75a570f9dc5 b645bb48850fea8db017026897827f0ab42fbdea fc81ba536bc3d8cdbcf9e92369e9bc5ede69da10 We've put it off for too long already.
Re: [Qemu-devel] [PATCH 3/4] virtio-blk: fix reference a pointer which might be freed
On Mon, Aug 04, 2014 at 04:25:43PM +0800, zhanghailiang wrote: In function virtio_blk_handle_request, it may freed memory pointed by req, So do not access member of req after calling this function. Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com --- hw/block/virtio-blk.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) Reviewed-by: Stefan Hajnoczi stefa...@redhat.com pgpGXraKxNIJ4.pgp Description: PGP signature
Re: [Qemu-devel] fpu/softfloat.c licensing
On 4 August 2014 12:12, Markus Armbruster arm...@redhat.com wrote: I'm afraid we need rip out the problematic patches. Here's Paolo's list: Fabrice Bellard fabr...@bellard.org 1d6bda356153c82e100680d9f2165e32c8fb1330 750afe93fd15fafc20b6c34d30f339547d15c2d1 Jocelyn Mayer 75d62a585629cdc1ae0d530189653cb1d8d9c53c Thiemo Seufer's parents (Stefan said he'd contact them) 5a6932d51d1b34b68b3f10fc5ac65598bece88c0 924b2c07cdfaba9ac408fc5fa77da75a570f9dc5 b645bb48850fea8db017026897827f0ab42fbdea fc81ba536bc3d8cdbcf9e92369e9bc5ede69da10 We've put it off for too long already. Yes; I'd like to get this sorted for the next release. I've started a couple of inquiries about whether Anthony's proposed plan is legally sensible and whether we need to get the bits that need reimplementing done in a clean room fashion or not. thanks -- PMM
Re: [Qemu-devel] [PATCH 1/2] virtio-serial: create a linked list of all active devices
Amit Shah amit.s...@redhat.com writes:
To ensure two virtserialports don't get added to the system with the
same 'name' parameter, we need to access all the ports on all the
devices added, and compare the names.
We currently don't have a list of all VirtIOSerial devices added to the
system. This commit adds a simple linked list in which devices are put
when they're initialized, and removed when they go away.
Signed-off-by: Amit Shah amit.s...@redhat.com
---
hw/char/virtio-serial-bus.c | 10 ++
include/hw/virtio/virtio-serial.h | 2 ++
2 files changed, 12 insertions(+)
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 07bebc0..8c26f4e 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -26,6 +26,10 @@
#include hw/virtio/virtio-serial.h
#include hw/virtio/virtio-access.h
+struct VirtIOSerialDevices {
+QLIST_HEAD(, VirtIOSerial) devices;
+} vserdevices;
+
Any particular reason for stuffing the list into a struct?
static VirtIOSerialPort *find_port_by_id(VirtIOSerial *vser, uint32_t id)
{
VirtIOSerialPort *port;
@@ -975,6 +979,8 @@ static void virtio_serial_device_realize(DeviceState
*dev, Error **errp)
*/
register_savevm(dev, virtio-console, -1, 3, virtio_serial_save,
virtio_serial_load, vser);
+
+QLIST_INSERT_HEAD(vserdevices.devices, vser, next);
}
static void virtio_serial_port_class_init(ObjectClass *klass, void *data)
@@ -1003,6 +1009,8 @@ static void virtio_serial_device_unrealize(DeviceState
*dev, Error **errp)
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
VirtIOSerial *vser = VIRTIO_SERIAL(dev);
+QLIST_REMOVE(vser, next);
+
unregister_savevm(dev, virtio-console, vser);
g_free(vser-ivqs);
@@ -1027,6 +1035,8 @@ static void virtio_serial_class_init(ObjectClass
*klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+QLIST_INIT(vserdevices.devices);
+
dc-props = virtio_serial_properties;
set_bit(DEVICE_CATEGORY_INPUT, dc-categories);
vdc-realize = virtio_serial_device_realize;
diff --git a/include/hw/virtio/virtio-serial.h
b/include/hw/virtio/virtio-serial.h
index 4746312..a679e54 100644
--- a/include/hw/virtio/virtio-serial.h
+++ b/include/hw/virtio/virtio-serial.h
@@ -202,6 +202,8 @@ struct VirtIOSerial {
QTAILQ_HEAD(, VirtIOSerialPort) ports;
+QLIST_ENTRY(VirtIOSerial) next;
+
/* bitmap for identifying active ports */
uint32_t *ports_map;
Patch looks simple safe to me, but I can't help to wonder whether want
(or already have?) more generic infrastructure offering for all devices
of a certain kind functionality, which is what 2/2 needs. Andreas?
Re: [Qemu-devel] [PATCH v2 0/5] AArch64 TLB performance improvements
Alex Bennée writes: Paolo Bonzini writes: Il 30/07/2014 17:20, Alex Bennée ha scritto: Hi, snip The most important thing is I've measured a 25-30% improvement in kernel and android boot time. snip Hi Alex, have you seen this patch? Perhaps you're interested in reviving it. http://article.gmane.org/gmane.comp.emulators.qemu/253864 I saw it when it first came out but I didn't quite follow what it was doing as I hadn't looked at the TLB code. I'll have another look and see what difference it can make. A quick and dirty benchmark: Comparing 10bit/12bit tables with and without [[http://article.gmane.org/gmane.comp.emulators.qemu/253864][victim cache]] #+BEGIN_NOTES Time in seconds, smaller is better Percentage is amount of time compared to run to the left #+END_NOTES | Code | 10 bit | 10 bit + victim |12 bit | 12 bit + victim | |---+--+-+---+-| | | 12.783 | 11.664 |10.348 | 9.527 | | Runs | 13.046 | 11.971 |10.123 | 9.326 | | | 12.929 | 11.673 |11.130 | 9.858 | | | 12.981 | 11.941 |10.223 | 9.673 | |---+--+-+---+-| | Avgs | 12.93475 |11.81225 |10.456 | 9.596 | |---+--+-+---+-| | %prev | 100% | 91.321827 | 88.518276 | 91.775057 | #+TBLFM: $2=vmean(@I..II)::$3=(@II$3/@II$2)*100::$4=vmean(@I..II)::$5=vmean(@I..II) Which as you expect shows the page table size is a greater improvement to the performance but the victim cache also improves the run time on top of this. I say as you would expect because any time you need to exit translated code there is a bunch of overhead in doing so. -- Alex Bennée
Re: [Qemu-devel] [PATCH 07/15] dataplane: use object pool to speed up allocation for virtio blk request
On Mon, Aug 4, 2014 at 6:21 PM, Stefan Hajnoczi stefa...@redhat.com wrote: On Fri, Aug 01, 2014 at 03:42:05PM +0800, Ming Lei wrote: On Thu, Jul 31, 2014 at 5:18 PM, Paolo Bonzini pbonz...@redhat.com wrote: Il 31/07/2014 05:22, Ming Lei ha scritto: The problem is that g_slice here is not using the slab-style allocator because the object is larger than roughly 500 bytes. One solution would be to make virtqueue_pop/vring_pop allocate a VirtQueueElement of the right size (and virtqueue_push/vring_push free it), as mentioned in the review of patch 8. Unluckily both iovec and addr array can't be fitted into 500 bytes, :-( Not mention all users of VirtQueueElement need to be changed too, I hate to make that work involved in this patchset, :-) Well, the point of dataplane was not just to get maximum iops. It was also to provide guidance in the work necessary to improve the code and get maximum iops without special-casing everything. This can be a lot of work indeed. However, I now remembered that VirtQueueElement is a mess because it's serialized directly into the migration state. :( So you basically cannot change it without mucking with migration. Please leave out patch 8 for now. save_device code serializes elem in this way: qemu_put_buffer(f, (unsigned char *)req-elem, sizeof(VirtQueueElement)); so I am wondering why this patch may break migration. Because you change the on-wire format and break migration from 2.1 to 2.2. Sorry, I wasn't clear enough. That is really a mess, but in future we still may convert VirtQueueElement into a smart one, and keep the original structure only for save/load, but a conversion between the two structures is required in save/load. This is a good idea. The VirtQueueElement structure isn't complicated, it's just big. Just use the current layout as the serialization format. The patch shouldn't be complicated too, and the only annoying part is to find each virtio device's load/save , and add the conversion in the two functions if VirtQueueElement is involved. I suggest to do the conversion in another standalone patchset. Thanks,
Re: [Qemu-devel] [PATCH 1/2] virtio-serial: create a linked list of all active devices
On (Mon) 04 Aug 2014 [13:33:56], Markus Armbruster wrote:
Amit Shah amit.s...@redhat.com writes:
To ensure two virtserialports don't get added to the system with the
same 'name' parameter, we need to access all the ports on all the
devices added, and compare the names.
We currently don't have a list of all VirtIOSerial devices added to the
system. This commit adds a simple linked list in which devices are put
when they're initialized, and removed when they go away.
snip
+struct VirtIOSerialDevices {
+QLIST_HEAD(, VirtIOSerial) devices;
+} vserdevices;
+
Any particular reason for stuffing the list into a struct?
Not strictly needed for this patch alone, but I think it's cleaner to
keep it this way, and when something else comes up that needs a
per-device variable, this list will be around. Also, this is also the
way it's done in the kernel, so that uniformity helps as well.
snip
Patch looks simple safe to me, but I can't help to wonder whether want
(or already have?) more generic infrastructure offering for all devices
of a certain kind functionality, which is what 2/2 needs. Andreas?
Yea; I didn't find any, but if there's already something it can be put
to good use here.
Thanks,
Amit
Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex command
Gonglei (Arei) arei.gong...@huawei.com writes:
Hi, Markus
Subject: Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex
command
On Fri, Jul 25, 2014 at 02:52:50PM +0800, arei.gong...@huawei.com
wrote:
From: Gonglei arei.gong...@huawei.com
Adds set-bootindex id=xx,bootindex=xx,suffix=xx QMP command.
Example QMP command:
- { execute: set-bootindex, arguments: { id: ide0-0-1,
bootindex:
1, suffix: /disk@0}}
- { return: {} }
Signed-off-by: Gonglei arei.gong...@huawei.com
Signed-off-by: Chenliang chenlian...@huawei.com
---
qapi-schema.json | 16
qmp-commands.hx | 24
qmp.c| 17 +
3 files changed, 57 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json
index b11aad2..a9ef0be 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1704,6 +1704,22 @@
{ 'command': 'device_del', 'data': {'id': 'str'} }
##
+# @set-bootindex:
+#
+# set bootindex of a devcie
+#
+# @id: the name of the device
+# @bootindex: the bootindex of the device
+# @suffix: #optional a suffix of the device
+#
+# Returns: Nothing on success
+# If @id is not a valid device, DeviceNotFound
+#
+# Since: 2.2
+##
+{ 'command': 'set-bootindex', 'data': {'id': 'str', 'bootindex':
int', '*suffix':
'str'} }
+
+##
I wonder if we could simply use qom-set for that. How many devices
actually support having multiple bootindex entries with different
suffixes?
AFAICT, the floppy device support two bootindex with different suffixes.
Block device models commonly a single block device. By convention,
property drive is the backend, and property bootindex the bootindex,
if the device model supports that.
The device ID suffices to identify a block device for such device
models.
The floppy device model is an exception. It folds multiple real-world
objects into one: the controller and the actual drives. Have a look at
-device isa-fdc,help:
isa-fdc.iobase=uint32
isa-fdc.irq=uint32
isa-fdc.dma=uint32
isa-fdc.driveA=drive
isa-fdc.driveB=drive
isa-fdc.bootindexA=int32
isa-fdc.bootindexB=int32
isa-fdc.check_media_rate=on/off
The properties ending with 'A' or 'B' apply to the first and the second
drive, the others to the controller.
Unfortunately, this means the device ID by itself doesn't identify the
floppy device.
Yes.
Arguably, this is lousy modeling --- we really should model separate
real-world objects as separate objects. But making floppies pretty (or
even sane) isn't anyone's priority nowadays.
Hmm... Agreed.
Since qom-set works on *properties*, I can't see why it couldn't be used
for changing bootindexes. There is no ambiguity even with floppy.
Sorry?
You either set bootindexA or bootindexB. No need to fuzz around with
suffixes. Or am I missing something?
Your mean that we just need to think about change one bootindex? Either
set bootindexA or bootindexB, do not need pass suffix for identifying?
Eduardo suggested to think about using qom-set to update the bootindex.
Could look like
{ execute: qom-set,
arguments: { path: /machine/unattached/device[15],
property: bootindexA, value: 1 } }
Demonstrates an existing, well-defined way to identify the bootindex to
change. Do we really want to invent another one, based on suffix?
The value of path is the QOM path. I can't remember offhand how to go
from qdev ID to QOM path. Onboard devices like isa-fdc don't have one
anyway.
I also don't remember whether there's a nicer QOM path than this one.
Thanks for your explaining. TBH I haven't used qom-set before, so I
Few people have :)
don't know what your mean (like Eduardo, sorry again).
But now, I have a look at the implement of qom-set command, and
I find the command is just change a device's property value, do not have
any other logic. For my case, we really change value of the global
fw_boot_order list,
which the device's bootindex property worked for actually. Only in this way,
we can attach our original target.
Why can't we delay the logic to the next reboot? Let me explain.
Current code starts with empty fw_boot_order, then lets device realize
add to it. Unplugging a device leaves a dangling DeviceState pointer in
the list (I think).
Could we instead build, use and free the list during reboot? That way,
qom-set of bootindex affects the next reboot without additional logic.
[Qemu-devel] [PATCH v5 1/2] loader: Add load_image_gzipped function.
As the name suggests this lets you load a ROM/disk image that is
gzipped. It is uncompressed before storing it in guest memory.
Signed-off-by: Richard W.M. Jones rjo...@redhat.com
---
hw/core/loader.c| 48
include/hw/loader.h | 1 +
2 files changed, 49 insertions(+)
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 2bf6b8f..e773aab 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -577,6 +577,54 @@ int load_ramdisk(const char *filename, hwaddr addr,
uint64_t max_sz)
return load_uboot_image(filename, NULL, addr, NULL, IH_TYPE_RAMDISK);
}
+/* This simply prevents g_malloc in the function below from allocating
+ * a huge amount of memory, by placing a limit on the maximum
+ * uncompressed image size that load_image_gzipped will read.
+ */
+#define LOAD_IMAGE_MAX_GUNZIP_BYTES (256 20)
+
+/* Load a gzip-compressed kernel. */
+int load_image_gzipped(const char *filename, hwaddr addr, uint64_t max_sz)
+{
+uint8_t *compressed_data = NULL;
+uint8_t *data = NULL;
+gsize len;
+ssize_t bytes;
+int ret = -1;
+
+if (!g_file_get_contents(filename, (char **) compressed_data, len,
+ NULL)) {
+goto out;
+}
+
+/* Is it a gzip-compressed file? */
+if (len 2 ||
+compressed_data[0] != '\x1f' ||
+compressed_data[1] != '\x8b') {
+goto out;
+}
+
+if (max_sz LOAD_IMAGE_MAX_GUNZIP_BYTES) {
+max_sz = LOAD_IMAGE_MAX_GUNZIP_BYTES;
+}
+
+data = g_malloc(max_sz);
+bytes = gunzip(data, max_sz, compressed_data, len);
+if (bytes 0) {
+fprintf(stderr, %s: unable to decompress gzipped kernel file\n,
+filename);
+goto out;
+}
+
+rom_add_blob_fixed(filename, data, bytes, addr);
+ret = bytes;
+
+ out:
+g_free(compressed_data);
+g_free(data);
+return ret;
+}
+
/*
* Functions for reboot-persistent memory regions.
* - used for vga bios and option roms.
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 796cbf9..00c9117 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -15,6 +15,7 @@ int get_image_size(const char *filename);
int load_image(const char *filename, uint8_t *addr); /* deprecated */
int load_image_targphys(const char *filename, hwaddr,
uint64_t max_sz);
+int load_image_gzipped(const char *filename, hwaddr addr, uint64_t max_sz);
#define ELF_LOAD_FAILED -1
#define ELF_LOAD_NOT_ELF -2
--
2.0.4
[Qemu-devel] [PATCH v5 2/2] aarch64: Allow -kernel option to take a gzip-compressed kernel.
On aarch64 it is the bootloader's job to uncompress the kernel. UEFI
and u-boot bootloaders do this automatically when the kernel is
gzip-compressed.
However the qemu -kernel option does not do this. The following
command does not work:
qemu-system-aarch64 [...] -kernel /boot/vmlinuz
because it tries to execute the gzip-compressed data.
This commit lets gzip-compressed kernels be uncompressed
transparently.
Currently this is only done when emulating aarch64.
Signed-off-by: Richard W.M. Jones rjo...@redhat.com
---
hw/arm/boot.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 3d1f4a2..1d541db 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -444,6 +444,7 @@ static void do_cpu_reset(void *opaque)
void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
{
CPUState *cs = CPU(cpu);
+int allow_compressed_kernels = 0;
int kernel_size;
int initrd_size;
int is_linux = 0;
@@ -465,6 +466,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
*info)
primary_loader = bootloader_aarch64;
kernel_load_offset = KERNEL64_LOAD_ADDR;
elf_machine = EM_AARCH64;
+allow_compressed_kernels = 1;
} else {
primary_loader = bootloader;
kernel_load_offset = KERNEL_LOAD_ADDR;
@@ -510,6 +512,13 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
*info)
kernel_size = load_uimage(info-kernel_filename, entry, NULL,
is_linux);
}
+/* On aarch64, it's the bootloader's job to uncompress the kernel. */
+if (allow_compressed_kernels kernel_size 0) {
+entry = info-loader_start + kernel_load_offset;
+kernel_size = load_image_gzipped(info-kernel_filename, entry,
+ info-ram_size - kernel_load_offset);
+is_linux = 1;
+}
if (kernel_size 0) {
entry = info-loader_start + kernel_load_offset;
kernel_size = load_image_targphys(info-kernel_filename, entry,
--
2.0.4
[Qemu-devel] [PATCH v5 0/2] aarch64: Allow -kernel option to take a gzip-compressed kernel.
Changes since v4: - Split the patch into a generic loader part, and specific arm64 support. - There is now a specific limit for the gzip loader, plus a comment to indicate that it's just there to stop an excessive malloc. The limit is now decoupled (and larger) than the u-boot limit, since this function might be used for non-kernels in future. - Remove comment about aarch64 in generic code. Rich.
Re: [Qemu-devel] [PULL 0/3] Xen tree 2014-08-01
On 1 August 2014 17:00, Stefano Stabellini stefano.stabell...@eu.citrix.com wrote: The following changes since commit 541bbb07eb197a870661ed702ae1f15c7d46aea6: Update version for v2.1.0 release (2014-08-01 13:31:29 +0100) are available in the git repository at: git://xenbits.xen.org/people/sstabellini/qemu-dm.git xen-20140801 for you to fetch changes up to b33a5bbfbaab6c1ce653a8e3665a18ca67de1456: qemu: support xen hvm direct kernel boot (2014-08-01 15:58:12 +) Chunyan Liu (1): qemu: support xen hvm direct kernel boot Roger Pau Monne (2): xen: fix usage of ENODATA tap-bsd: implement a FreeBSD only version of tap_open Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex command
Hi, Markus
Subject: Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex
command
On Fri, Jul 25, 2014 at 02:52:50PM +0800, arei.gong...@huawei.com
wrote:
From: Gonglei arei.gong...@huawei.com
Adds set-bootindex id=xx,bootindex=xx,suffix=xx QMP command.
Example QMP command:
- { execute: set-bootindex, arguments: { id: ide0-0-1,
bootindex:
1, suffix: /disk@0}}
- { return: {} }
Signed-off-by: Gonglei arei.gong...@huawei.com
Signed-off-by: Chenliang chenlian...@huawei.com
---
qapi-schema.json | 16
qmp-commands.hx | 24
qmp.c| 17 +
3 files changed, 57 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json
index b11aad2..a9ef0be 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1704,6 +1704,22 @@
{ 'command': 'device_del', 'data': {'id': 'str'} }
##
+# @set-bootindex:
+#
+# set bootindex of a devcie
+#
+# @id: the name of the device
+# @bootindex: the bootindex of the device
+# @suffix: #optional a suffix of the device
+#
+# Returns: Nothing on success
+# If @id is not a valid device, DeviceNotFound
+#
+# Since: 2.2
+##
+{ 'command': 'set-bootindex', 'data': {'id': 'str', 'bootindex':
int', '*suffix':
'str'} }
+
+##
I wonder if we could simply use qom-set for that. How many devices
actually support having multiple bootindex entries with different
suffixes?
AFAICT, the floppy device support two bootindex with different
suffixes.
Block device models commonly a single block device. By convention,
property drive is the backend, and property bootindex the bootindex,
if the device model supports that.
The device ID suffices to identify a block device for such device
models.
The floppy device model is an exception. It folds multiple real-world
objects into one: the controller and the actual drives. Have a look at
-device isa-fdc,help:
isa-fdc.iobase=uint32
isa-fdc.irq=uint32
isa-fdc.dma=uint32
isa-fdc.driveA=drive
isa-fdc.driveB=drive
isa-fdc.bootindexA=int32
isa-fdc.bootindexB=int32
isa-fdc.check_media_rate=on/off
The properties ending with 'A' or 'B' apply to the first and the second
drive, the others to the controller.
Unfortunately, this means the device ID by itself doesn't identify the
floppy device.
Yes.
Arguably, this is lousy modeling --- we really should model separate
real-world objects as separate objects. But making floppies pretty (or
even sane) isn't anyone's priority nowadays.
Hmm... Agreed.
Since qom-set works on *properties*, I can't see why it couldn't be used
for changing bootindexes. There is no ambiguity even with floppy.
Sorry?
You either set bootindexA or bootindexB. No need to fuzz around with
suffixes. Or am I missing something?
Your mean that we just need to think about change one bootindex? Either
set bootindexA or bootindexB, do not need pass suffix for identifying?
Eduardo suggested to think about using qom-set to update the bootindex.
Could look like
{ execute: qom-set,
arguments: { path: /machine/unattached/device[15],
property: bootindexA, value: 1 } }
Demonstrates an existing, well-defined way to identify the bootindex to
change. Do we really want to invent another one, based on suffix?
The value of path is the QOM path. I can't remember offhand how to go
from qdev ID to QOM path. Onboard devices like isa-fdc don't have one
anyway.
I also don't remember whether there's a nicer QOM path than this one.
Thanks for your explaining. TBH I haven't used qom-set before, so I
Few people have :)
don't know what your mean (like Eduardo, sorry again).
But now, I have a look at the implement of qom-set command, and
I find the command is just change a device's property value, do not have
any other logic. For my case, we really change value of the global
fw_boot_order list,
which the device's bootindex property worked for actually. Only in this way,
we can attach our original target.
Why can't we delay the logic to the next reboot? Let me explain.
Current code starts with empty fw_boot_order, then lets device realize
add to it. Unplugging a device leaves a dangling DeviceState pointer in
the list (I think).
Yes.
Could we instead build, use and free the list during reboot? That way,
qom-set of bootindex affects the next reboot without additional logic.
Yes, we can do this, but it will be too complicated. Firstly the device realize
will not reattach during reboot. So, we should check all the device of the
global list during reboot,
but the DeviceState haven't
Re: [Qemu-devel] Question of emulation on MSR's in KVM-mode
Il 04/08/2014 10:37, Morty Andersen ha scritto: Hi I'm working on an extension to QEMU (target i386). This involves adding new MSR's. I've got it working in non-KVM mode by adding these MSR's to the state and adding extra cases to helper_wrmsr(), helper_rdmsr(). The guest can now read/write these MSR's as expected. However, it fails when running in KVM-mode. Specifically, writing the MSR's causes GPF. Note that these MSR's are not natively supported by the host CPU. I don't know enough about Intel's VMX to tell if it is even reasonable to expect that this could work for a non-natively supported MSR. As far as I can read in the VMX documentation, the hypervisor can setup a bitmap of which MSR's should cause trap's to the hypervisor and which shouldn't. I guess it would be the KVM kernel module that does this based on input it receives from QEMU. But I haven't been able to find the part of QEMU that negotiates this. I guess the solution for me is to set the necessary bits to that access to the new MSR's causes traps. Next, I need to add/modify the trap handler so that it can handle the MSR's. Hi, handling of the MSRs in KVM is done entirely in the hypervisor. QEMU only gets/sets them in order to support migration. You need to modify the KVM kernel module for the VM to recognize your special MSRs. Paolo
Re: [Qemu-devel] [PATCH 0/3] some numa memory related fixes
On Mon, Aug 04, 2014 at 04:16:06PM +0800, Hu Tao wrote: See each patch for the detail. Hu Tao (3): hw:i386: typo fix: MEMORY_HOPTLUG_DEVICE - MEMORY_HOTPLUG_DEVICE pc-dimm: check if node property exceeds available numa nodes numa: show hex number in error message for consistency and prefix them with 0x Thanks, applied. hw/i386/acpi-dsdt.dsl | 4 ++-- hw/i386/acpi-dsdt.hex.generated | 8 hw/i386/q35-acpi-dsdt.dsl | 4 ++-- hw/i386/ssdt-mem.dsl| 16 hw/i386/ssdt-misc.dsl | 2 +- hw/mem/pc-dimm.c| 5 + include/hw/acpi/pc-hotplug.h| 2 +- numa.c | 4 ++-- 8 files changed, 25 insertions(+), 20 deletions(-) -- 1.9.3
Re: [Qemu-devel] [PATCH 2/3] pc-dimm: check if the value of node property
On Mon, Aug 04, 2014 at 04:16:08PM +0800, Hu Tao wrote:
If user specifies a node number that exceeds the available numa nodes in
emulated system for pc-dimm device, the device will reports an invalid _PXM
to OSPM. Fix it by checking the node value.
Signed-off-by: Hu Tao hu...@cn.fujitsu.com
---
hw/mem/pc-dimm.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 08f49ed..92e276f 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -252,6 +252,11 @@ static void pc_dimm_realize(DeviceState *dev, Error
**errp)
error_setg(errp, ' PC_DIMM_MEMDEV_PROP ' property is not set);
return;
}
+if (dimm-node = nb_numa_nodes) {
+error_setg(errp, ' PC_DIMM_NODE_PROP
+ ' exceeds numa node number: % PRId32, nb_numa_nodes);
PRId32 is wrong here, this variable is int, use %d.
Also, this message isn't very clear, I fixed it up
with a patch on top.
+return;
+}
}
static MemoryRegion *pc_dimm_get_memory_region(PCDIMMDevice *dimm)
--
1.9.3
Re: [Qemu-devel] [PATCH target-arm] arm: armv7m: Respect elf entry point
On 2 August 2014 00:41, Peter Crosthwaite crosthwaitepe...@gmail.com wrote:
ARMv7M has it's own bootloader (separate from the regular ARM
bootloader) that is elf aware. It is able to load elfs but it does
not set the program counter to the elf entry point. Make it more
consistent with the regular ARM bootloader by setting the program
counter to the given elf entry point.
Signed-off-by: Peter Crosthwaite crosthwaite.pe...@gmail.com
---
hw/arm/armv7m.c | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 397e8df..d1b983f 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -155,11 +155,18 @@ static void armv7m_bitband_init(void)
/* Board init. */
+typedef struct ARMV7MResetArgs {
+ARMCPU *cpu;
+uint32_t reset_pc;
+} ARMV7MResetArgs;
+
static void armv7m_reset(void *opaque)
{
-ARMCPU *cpu = opaque;
+ARMV7MResetArgs *args = opaque;
-cpu_reset(CPU(cpu));
+cpu_reset(CPU(args-cpu));
+args-cpu-env.regs[15] = args-reset_pc;
+args-cpu-env.thumb = args-reset_pc 1;
This looks odd. If the entry point has bit 0 being the
Thumb bit, then shouldn't we be masking it out the
same way we do in the A-profile do_cpu_reset() ?
}
/* Init CPU and memory for a v7-M based board.
@@ -183,6 +190,7 @@ qemu_irq *armv7m_init(MemoryRegion *address_space_mem,
MemoryRegion *sram = g_new(MemoryRegion, 1);
MemoryRegion *flash = g_new(MemoryRegion, 1);
MemoryRegion *hack = g_new(MemoryRegion, 1);
+ARMV7MResetArgs reset_args;
flash_size *= 1024;
sram_size *= 1024;
@@ -259,7 +267,12 @@ qemu_irq *armv7m_init(MemoryRegion *address_space_mem,
vmstate_register_ram_global(hack);
memory_region_add_subregion(address_space_mem, 0xf000, hack);
-qemu_register_reset(armv7m_reset, cpu);
+reset_args = (ARMV7MResetArgs) {
+.cpu = cpu,
+.reset_pc = entry,
+};
+qemu_register_reset(armv7m_reset,
+g_memdup(reset_args, sizeof(reset_args)));
Why the local variable and memdup rather than just
g_new0() and set the fields in the allocated struct?
thanks
-- PMM
Re: [Qemu-devel] [PATCH v2 01/10] target-arm/cpu.h: document various program state functions
On 10 July 2014 16:49, Alex Bennée alex.ben...@linaro.org wrote:
We have a number of program state saving functions (pstate, cpsr, xpsr)
which are dependant on the mode the CPU is in. This commit adds a little
documentation to each function and asserts to defend against incorrect
use.
Signed-off-by: Alex Bennée alex.ben...@linaro.org
---
v2:
- remove xpsr_state asserts
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 369d472..c2312d0 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -475,22 +475,34 @@ int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr
address, int rw,
#define PSTATE_MODE_EL1t 4
#define PSTATE_MODE_EL0t 0
-/* Return the current PSTATE value. For the moment we don't support 32-64
bit
- * interprocessing, so we don't attempt to sync with the cpsr state used by
- * the 32 bit decoder.
+/* ARMv8 ARM D1.7 Process state, PSTATE
+ *
+ * 31 28 27 24 23 22 21 20 2221 20 19 16 15 8 7 5 40
+ * +--+--+---+-++---+--+--+-+--+
+ * | NZCV | DAIF | SS IL | EL | nRW SP | Q | GE | IT | JTE | Mode |
+ * +--+--+---+-++---+--+--+-+--+
This is a bit misleading because the ARM ARM very deliberately
doesn't define a bit format for PSTATE. I suspect what we're actually
dealing with here is the PSTATE in SPSR format.
+ *
+ * The PSTATE is an abstraction of a number of Return the current
Mangled comment text?
+ * PSTATE value. This is only valid for A64 hardware although can be
+ * read when in AArch32 mode.
*/
static inline uint32_t pstate_read(CPUARMState *env)
{
int ZF;
+g_assert(is_a64(env));
+
ZF = (env-ZF == 0);
return (env-NF 0x8000) | (ZF 30)
| (env-CF 29) | ((env-VF 0x8000) 3)
| env-pstate | env-daif;
}
+/* Update the current PSTATE value. This doesn't include nRW which is */
Another truncated comment.
static inline void pstate_write(CPUARMState *env, uint32_t val)
{
+g_assert(is_a64(env));
+
env-ZF = (~val) PSTATE_Z;
env-NF = val;
env-CF = (val 29) 1;
@@ -499,15 +511,22 @@ static inline void pstate_write(CPUARMState *env,
uint32_t val)
env-pstate = val ~CACHED_PSTATE_BITS;
}
-/* Return the current CPSR value. */
+/* ARMv7-AR ARM B1.3.3 Current Program Status Register, CPSR
+ *
+ * Unlike the above PSTATE implementation these functions will attempt
+ * to switch processor mode when the M[4:0] bits are set.
+ */
uint32_t cpsr_read(CPUARMState *env);
/* Set the CPSR. Note that some bits of mask must be all-set or all-clear.
*/
void cpsr_write(CPUARMState *env, uint32_t val, uint32_t mask);
-/* Return the current xPSR value. */
+/* ARMv7-M ARM B1.4.2, special purpose program status register xPSR */
static inline uint32_t xpsr_read(CPUARMState *env)
{
int ZF;
+
+g_assert(!is_a64(env));
+
ZF = (env-ZF == 0);
return (env-NF 0x8000) | (ZF 30)
| (env-CF 29) | ((env-VF 0x8000) 3) | (env-QF 27)
@@ -519,6 +538,8 @@ static inline uint32_t xpsr_read(CPUARMState *env)
/* Set the xPSR. Note that some bits of mask must be all-set or all-clear.
*/
static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
{
+g_assert(!is_a64(env));
+
if (mask CPSR_NZCV) {
env-ZF = (~val) CPSR_Z;
env-NF = val;
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 2b4ce6a..ec1fef5 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -506,8 +506,8 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
env-condexec_bits = 0;
}
-pstate_write(env, PSTATE_DAIF | PSTATE_MODE_EL1h);
env-aarch64 = 1;
+pstate_write(env, PSTATE_DAIF | PSTATE_MODE_EL1h);
env-pc = addr;
cs-interrupt_request |= CPU_INTERRUPT_EXITTB;
--
2.0.1
thanks
-- PMM
Re: [Qemu-devel] [PATCH 4/4] ivshmem: check the value returned by fstat()
On Mon, Aug 04, 2014 at 04:25:44PM +0800, zhanghailiang wrote:
The function fstat() may fail, so check its return value.
Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com
---
hw/misc/ivshmem.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 768e528..2667e9f 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -324,7 +324,10 @@ static int check_shm_size(IVShmemState *s, int fd) {
struct stat buf;
-fstat(fd, buf);
+if (fstat(fd, buf) 0) {
+fprintf(stderr, Cannot stat IVSHMEM: %s\n, strerror(errno));
+return -1;
+}
That's a confusing error message:
1. You don't stat ivshmem. You stat a shmem fd. Also best to print fd #.
2. Tell the user what action was taken, e.g. IVSHMEM failed to start.
if (s-ivshmem_size buf.st_size) {
fprintf(stderr,
--
1.7.12.4
Re: [Qemu-devel] vfio in the guest: no available reset mechanism
On Sat, Aug 02, 2014 at 07:54:30AM +0200, Jan Kiszka wrote: On 2014-08-01 19:16, Alex Williamson wrote: Also, it may let some of our device models deviate from their real versions (I suppose, e.g., none of the e1000 devices we currently emulate exposed FLR). Of course, but what are the chances that the driver will care? No drivers of GPOSes, but special or legacy OSes may do so. Keep in mind that Intel e.g. is documenting their PCI devices with fixed config space addresses for their capability. Also, we completely lack PM caps so far. Adding them would already have the value of increasing emulation accuracy. And here I think we are free to always implement reset behavior behind D3-D0 transitions. So my believe is that this will be the better path. Jan That's true but is PM reset per-function or per-device? I'll have to check the spec - do you happen to rememeber?
Re: [Qemu-devel] [PATCH v2 02/10] target-arm/cpu.h: common pstate save/restore
On 10 July 2014 16:49, Alex Bennée alex.ben...@linaro.org wrote:
This adds a universal program state save and restore function. This is
intended to simplify the migration serialisation functionality and avoid
special casing depending on the mode of the CPU at serialisation time.
Signed-off-by: Alex Bennée alex.ben...@linaro.org
---
v2:
- reword commentary for restore_state_from_spsr
- rm commented out code
- add set_condition_codes for flags
- add xpsr_read functionality
v3:
- rebase
- checkpatch.pl issues
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index c2312d0..fe4d4f3 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -453,19 +453,24 @@ int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr
address, int rw,
#define PSTATE_SP (1U)
#define PSTATE_M (0xFU)
#define PSTATE_nRW (1U 4)
+#define PSTATE_T (1U 5)
#define PSTATE_F (1U 6)
#define PSTATE_I (1U 7)
#define PSTATE_A (1U 8)
#define PSTATE_D (1U 9)
#define PSTATE_IL (1U 20)
#define PSTATE_SS (1U 21)
+#define PSTATE_Q (1U 27)
#define PSTATE_V (1U 28)
#define PSTATE_C (1U 29)
#define PSTATE_Z (1U 30)
#define PSTATE_N (1U 31)
#define PSTATE_NZCV (PSTATE_N | PSTATE_Z | PSTATE_C | PSTATE_V)
-#define PSTATE_DAIF (PSTATE_D | PSTATE_A | PSTATE_I | PSTATE_F)
-#define CACHED_PSTATE_BITS (PSTATE_NZCV | PSTATE_DAIF)
+#define PSTATE_AIF (PSTATE_A | PSTATE_I | PSTATE_F)
+#define PSTATE_DAIF (PSTATE_D | PSTATE_AIF)
+#define AARCH64_CACHED_PSTATE_BITS (PSTATE_NZCV | PSTATE_DAIF)
+#define AARCH32_CACHED_PSTATE_BITS (PSTATE_NZCV | PSTATE_Q | PSTATE_AIF \
+| CACHED_CPSR_BITS)
/* Mode values for AArch64 */
#define PSTATE_MODE_EL3h 13
#define PSTATE_MODE_EL3t 12
@@ -508,7 +513,7 @@ static inline void pstate_write(CPUARMState *env,
uint32_t val)
env-CF = (val 29) 1;
env-VF = (val 3) 0x8000;
env-daif = val PSTATE_DAIF;
-env-pstate = val ~CACHED_PSTATE_BITS;
+env-pstate = val ~AARCH64_CACHED_PSTATE_BITS;
}
/* ARMv7-AR ARM B1.3.3 Current Program Status Register, CPSR
@@ -698,6 +703,137 @@ static inline bool arm_el_is_aa64(CPUARMState *env, int
el)
return arm_feature(env, ARM_FEATURE_AARCH64);
}
+/* ARMv8 ARM D1.6.4, Saved Program Status Registers
+ *
+ * These are formats used to save program status when exceptions are
+ * taken. There are two forms:
+ *
+ * AArch64 - AArch64 Exception
+ * 31 30 28 29 27 22 21 20 19 10 9 8 7 6 5 4 3 0
+ * +---+---+---+---+--+++--+---+---+---+---+---++
+ * | N | Z | C | V | RES0 | SS | IL | RES0 | D | A | I | F | 0 | 0 | M[3:0] |
+ * +---+---+---+---+--+++--+---+---+---+---+---++
+ *
+ * AArch32 - AArch64 Exception
+ * (see also ARMv7-AR ARM B1.3.3 CSPR/SPSR formats)
+ * 31 30 29 28 27 26 25 24 23 22 21 20 19 16
+ * +---+---+---+---+---+-+---+--+++-+
+ * | N | Z | C | V | Q | IT[1:0] | J | RES0 | SS | IL | GE[3:0] |
+ * +---+---+---+---+---+-+---+--+++-+
+ * 15 10 9 8 7 6 5 4 3 0
+ * +-+---+---+---+---+---+---++
+ * | IT[7:2] | E | A | I | F | T | 1 | M[3:0] |
+ * +-+---+---+---+---+---+---++
+ *
+ * M[4] = nRW - 0 = 64bit, 1 = 32bit
+ * Bit definitions for ARMv8 SPSR (PSTATE) format.
+ * Only these are valid when in AArch64 mode; in
+ * AArch32 mode SPSRs are basically CPSR-format.
+ *
+ * Also ARMv7-M ARM B1.4.2, special purpose program status register xPSR
+ */
+
+/* Save the program state */
+static inline uint32_t save_state_to_spsr(CPUARMState *env)
+{
+uint32_t final_spsr = 0;
+
+/* Calculate integer flags */
+final_spsr |= (env-NF 0x8000) ? PSTATE_N : 0; /* bit 31 */
+final_spsr |= (env-ZF == 0) ? PSTATE_Z : 0;
+final_spsr |= env-CF ? PSTATE_C : 0;/* or env-CF 29
*/
+final_spsr |= (env-VF 0x8000) ? PSTATE_V : 0; /* bit 31 */
+
+if (is_a64(env)) {
+/* SS - Software Step */
+/* IL - Illegal execution state */
Not sure you need specific comments here for SS and IL -- I
would expect those to live in env-pstate with the other
uncached bits.
+/* DAIF flags - should we mask?*/
+final_spsr |= (env-daif PSTATE_DAIF);
We don't mask env-daif in pstate_read() so we don't need to
here either.
+/* And the final un-cached bits */
+final_spsr |= (env-pstate ~AARCH64_CACHED_PSTATE_BITS);
+} else {
+/* condexec_bits is split over two parts */
+final_spsr |= ((env-condexec_bits 3) 25);
+final_spsr |= ((env-condexec_bits 0xfc) 8);
+
+if (arm_feature(env, ARM_FEATURE_M)) {
+final_spsr |= (env-thumb 24); /* alt pos */
This comment is too cryptic.
+final_spsr |= env-v7m.exception;
+} else {
+final_spsr |=
[Qemu-devel] [PATCH v5 1/8] bootindex: add modify_boot_device_path function
From: Gonglei arei.gong...@huawei.com
When we want to change one device's bootindex, we
should lookup the device and change the bootindex.
it is simply that remove it from the global boot list,
then re-add it, sorted by new bootindex.
If the new bootindex has already used by another device
just throw an error.
Allow changing the existing bootindex entry only,
not support adding new boot entries.
Signed-off-by: Gonglei arei.gong...@huawei.com
Signed-off-by: Chenliang chenlian...@huawei.com
---
include/sysemu/sysemu.h | 2 ++
vl.c| 66 +
2 files changed, 68 insertions(+)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index d8539fd..06e1b72 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -209,6 +209,8 @@ void usb_info(Monitor *mon, const QDict *qdict);
void add_boot_device_path(int32_t bootindex, DeviceState *dev,
const char *suffix);
+void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
+ const char *suffix, Error **errp);
char *get_boot_devices_list(size_t *size, bool ignore_suffixes);
DeviceState *get_boot_device(uint32_t position);
diff --git a/vl.c b/vl.c
index fe451aa..770ad67 100644
--- a/vl.c
+++ b/vl.c
@@ -1248,6 +1248,72 @@ void add_boot_device_path(int32_t bootindex, DeviceState
*dev,
QTAILQ_INSERT_TAIL(fw_boot_order, node, link);
}
+static bool is_same_fw_dev_path(DeviceState *src, DeviceState *dst)
+{
+bool ret = false;
+char *devpath_src = qdev_get_fw_dev_path(src);
+char *devpath_dst = qdev_get_fw_dev_path(dst);
+
+if (!strcmp(devpath_src, devpath_dst)) {
+ret = true;
+}
+
+g_free(devpath_src);
+g_free(devpath_dst);
+return ret;
+}
+
+void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
+ const char *suffix, Error **errp)
+{
+FWBootEntry *i, *old_entry = NULL;
+
+assert(dev != NULL || suffix != NULL);
+
+if (bootindex = 0) {
+QTAILQ_FOREACH(i, fw_boot_order, link) {
+if (i-bootindex == bootindex) {
+error_setg(errp, The bootindex %d has already been used,
+ bootindex);
+return;
+}
+}
+}
+
+QTAILQ_FOREACH(i, fw_boot_order, link) {
+/*
+ * Delete the same original dev, if devices havn't id property,
+ * we must check they fw_dev_path to identify them.
+ */
+if ((i-dev-id !strcmp(i-dev-id, dev-id)) ||
+(!i-dev-id is_same_fw_dev_path(i-dev, dev))) {
+if (!suffix) {
+QTAILQ_REMOVE(fw_boot_order, i, link);
+old_entry = i;
+
+break;
+} else if (i-suffix !strcmp(suffix, i-suffix)) {
+QTAILQ_REMOVE(fw_boot_order, i, link);
+old_entry = i;
+
+break;
+}
+}
+}
+
+if (!old_entry) {
+error_setg(errp, The device(%s) havn't configured bootindex property
+or suffix don't match, dev-id);
+
+return;
+}
+
+add_boot_device_path(bootindex, dev, old_entry-suffix);
+
+g_free(old_entry-suffix);
+g_free(old_entry);
+}
+
DeviceState *get_boot_device(uint32_t position)
{
uint32_t counter = 0;
--
1.7.12.4
[Qemu-devel] [PATCH v5 2/8] bootindex: add del_boot_device_path function
From: Gonglei arei.gong...@huawei.com
Introduce del_boot_device_path() to clean up fw_cfg content when
hot-unplugging a device that refers to a bootindex.
Signed-off-by: Gonglei arei.gong...@huawei.com
Signed-off-by: Chenliang chenlian...@huawei.com
---
include/sysemu/sysemu.h | 1 +
vl.c| 20
2 files changed, 21 insertions(+)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 06e1b72..130f971 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -209,6 +209,7 @@ void usb_info(Monitor *mon, const QDict *qdict);
void add_boot_device_path(int32_t bootindex, DeviceState *dev,
const char *suffix);
+void del_boot_device_path(DeviceState *dev);
void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
const char *suffix, Error **errp);
char *get_boot_devices_list(size_t *size, bool ignore_suffixes);
diff --git a/vl.c b/vl.c
index 770ad67..2424135 100644
--- a/vl.c
+++ b/vl.c
@@ -1263,6 +1263,26 @@ static bool is_same_fw_dev_path(DeviceState *src,
DeviceState *dst)
return ret;
}
+void del_boot_device_path(DeviceState *dev)
+{
+FWBootEntry *i;
+
+assert(dev != NULL);
+
+/* remove all entries of the assigned device */
+QTAILQ_FOREACH(i, fw_boot_order, link) {
+if ((i-dev-id dev-id
+!strcmp(i-dev-id, dev-id)) ||
+(!i-dev-id is_same_fw_dev_path(i-dev, dev))) {
+QTAILQ_REMOVE(fw_boot_order, i, link);
+g_free(i-suffix);
+g_free(i);
+
+break;
+}
+}
+}
+
void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
const char *suffix, Error **errp)
{
--
1.7.12.4
[Qemu-devel] [PATCH v5 5/8] qmp: add set-bootindex command
From: Gonglei arei.gong...@huawei.com
Adds set-bootindex id=xx,bootindex=xx,suffix=xx QMP command.
Example QMP command:
- { execute: set-bootindex,
arguments: { id: ide0-0-1, bootindex: 1, suffix: /disk@0}}
- { return: {} }
Signed-off-by: Gonglei arei.gong...@huawei.com
Signed-off-by: Chenliang chenlian...@huawei.com
---
qapi-schema.json | 17 +
qmp-commands.hx | 25 +
qmp.c| 17 +
3 files changed, 59 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json
index b11aad2..30bd6ad 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1704,6 +1704,23 @@
{ 'command': 'device_del', 'data': {'id': 'str'} }
##
+# @set-bootindex:
+#
+# set bootindex of a device
+#
+# @id: the name of the device
+# @bootindex: the bootindex of the device
+# @suffix: #optional a suffix of the device
+#
+# Returns: Nothing on success
+# If @id is not a valid device, DeviceNotFound
+#
+# Since: 2.2
+##
+{ 'command': 'set-bootindex',
+ 'data': {'id': 'str', 'bootindex': 'int', '*suffix': 'str'} }
+
+##
# @DumpGuestMemoryFormat:
#
# An enumeration of guest-memory-dump's format.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 4be4765..19cc3b8 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -330,6 +330,31 @@ Example:
- { return: {} }
EQMP
+{
+.name = set-bootindex,
+.args_type = id:s,bootindex:l,suffix:s?,
+.mhandler.cmd_new = qmp_marshal_input_set_bootindex,
+},
+
+SQMP
+set-bootindex
+-
+
+Set bootindex of a device
+
+Arguments:
+
+- id: the device's ID (json-string)
+- bootindex: the device's bootindex (json-int)
+- suffix: the device's suffix in global boot list (json-string, optional)
+
+Example:
+
+- { execute: set-bootindex,
+ arguments: { id: ide0-0-1, bootindex: 1, suffix: /disk@0}}
+- { return: {} }
+
+EQMP
{
.name = send-key,
diff --git a/qmp.c b/qmp.c
index 0d2553a..a375449 100644
--- a/qmp.c
+++ b/qmp.c
@@ -684,6 +684,23 @@ void qmp_object_del(const char *id, Error **errp)
object_unparent(obj);
}
+void qmp_set_bootindex(const char *id, int64_t bootindex,
+ bool has_suffix, const char *suffix, Error **errp)
+{
+DeviceState *dev;
+
+dev = qdev_find_recursive(sysbus_get_default(), id);
+if (!dev) {
+error_set(errp, QERR_DEVICE_NOT_FOUND, id);
+return;
+}
+if (has_suffix) {
+modify_boot_device_path(bootindex, dev, suffix, errp);
+} else {
+modify_boot_device_path(bootindex, dev, NULL, errp);
+}
+}
+
MemoryDeviceInfoList *qmp_query_memory_devices(Error **errp)
{
MemoryDeviceInfoList *head = NULL;
--
1.7.12.4
[Qemu-devel] [PATCH v5 4/8] bootindex: delete bootindex when device is removed
From: Gonglei arei.gong...@huawei.com
Device should be removed from global boot list when
it is hot-unplugged.
Signed-off-by: Gonglei arei.gong...@huawei.com
Signed-off-by: Chenliang chenlian...@huawei.com
---
hw/core/qdev.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index da1ba48..70294ad 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -972,6 +972,10 @@ static void device_finalize(Object *obj)
NamedGPIOList *ngl, *next;
DeviceState *dev = DEVICE(obj);
+
+/* remove device's bootindex from global boot order list */
+del_boot_device_path(dev);
+
if (dev-opts) {
qemu_opts_del(dev-opts);
}
--
1.7.12.4
[Qemu-devel] [PATCH v5 3/8] fw_cfg: add fw_cfg_machine_reset function
From: Gonglei arei.gong...@huawei.com
We must assure that the changed bootindex can take effect
when guest is rebooted. So we introduce fw_cfg_machine_reset(),
which change the fw_cfg file's bootindex data using the new
global fw_boot_order list.
Signed-off-by: Chenliang chenlian...@huawei.com
Signed-off-by: Gonglei arei.gong...@huawei.com
---
hw/nvram/fw_cfg.c | 54 +--
include/hw/nvram/fw_cfg.h | 2 ++
2 files changed, 49 insertions(+), 7 deletions(-)
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index b71d251..a24a44d 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -56,7 +56,6 @@ struct FWCfgState {
FWCfgFiles *files;
uint16_t cur_entry;
uint32_t cur_offset;
-Notifier machine_ready;
};
#define JPG_FILE 0
@@ -402,6 +401,26 @@ static void fw_cfg_add_bytes_read_callback(FWCfgState *s,
uint16_t key,
s-entries[arch][key].callback_opaque = callback_opaque;
}
+static void *fw_cfg_modify_bytes_read(FWCfgState *s, uint16_t key,
+ void *data, size_t len)
+{
+void *ptr;
+int arch = !!(key FW_CFG_ARCH_LOCAL);
+
+key = FW_CFG_ENTRY_MASK;
+
+assert(key FW_CFG_MAX_ENTRY len UINT32_MAX);
+
+/* return the old data to the function caller, avoid memory leak */
+ptr = s-entries[arch][key].data;
+s-entries[arch][key].data = data;
+s-entries[arch][key].len = len;
+s-entries[arch][key].callback_opaque = NULL;
+s-entries[arch][key].callback = NULL;
+
+return ptr;
+}
+
void fw_cfg_add_bytes(FWCfgState *s, uint16_t key, void *data, size_t len)
{
fw_cfg_add_bytes_read_callback(s, key, NULL, NULL, data, len);
@@ -499,13 +518,36 @@ void fw_cfg_add_file(FWCfgState *s, const char *filename,
fw_cfg_add_file_callback(s, filename, NULL, NULL, data, len);
}
-static void fw_cfg_machine_ready(struct Notifier *n, void *data)
+void *fw_cfg_modify_file(FWCfgState *s, const char *filename,
+void *data, size_t len)
+{
+int i, index;
+
+assert(s-files);
+
+index = be32_to_cpu(s-files-count);
+assert(index FW_CFG_FILE_SLOTS);
+
+for (i = 0; i index; i++) {
+if (strcmp(filename, s-files-f[i].name) == 0) {
+return fw_cfg_modify_bytes_read(s, FW_CFG_FILE_FIRST + i,
+ data, len);
+}
+}
+/* add new one */
+fw_cfg_add_file_callback(s, filename, NULL, NULL, data, len);
+return NULL;
+}
+
+static void fw_cfg_machine_reset(void *opaque)
{
+void *ptr;
size_t len;
-FWCfgState *s = container_of(n, FWCfgState, machine_ready);
+FWCfgState *s = opaque;
char *bootindex = get_boot_devices_list(len, false);
-fw_cfg_add_file(s, bootorder, (uint8_t*)bootindex, len);
+ptr = fw_cfg_modify_file(s, bootorder, (uint8_t *)bootindex, len);
+g_free(ptr);
}
FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
@@ -542,9 +584,7 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t
data_port,
fw_cfg_bootsplash(s);
fw_cfg_reboot(s);
-s-machine_ready.notify = fw_cfg_machine_ready;
-qemu_add_machine_init_done_notifier(s-machine_ready);
-
+qemu_register_reset(fw_cfg_machine_reset, s);
return s;
}
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index 72b1549..56e1ed7 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -76,6 +76,8 @@ void fw_cfg_add_file(FWCfgState *s, const char *filename,
void *data,
void fw_cfg_add_file_callback(FWCfgState *s, const char *filename,
FWCfgReadCallback callback, void
*callback_opaque,
void *data, size_t len);
+void *fw_cfg_modify_file(FWCfgState *s, const char *filename, void *data,
+ size_t len);
FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
hwaddr crl_addr, hwaddr data_addr);
--
1.7.12.4
[Qemu-devel] [PATCH v5 0/8] modify boot order of guest, and take effect after rebooting
From: Gonglei arei.gong...@huawei.com Sometimes, we want to modify boot order of a guest, but no need to shutdown it. We can call dynamic changing bootindex of a guest, which can be assured taking effect just after the guest rebooting. For example, in P2V scene, we boot a guest and then attach a new system disk, for copying some thing. We want to assign the new disk as the booting disk, which means its bootindex=1. Different nics can be assigen different bootindex dynamically also make sense. The patchsets add one qmp interface, and add an fw_cfg_machine_reset() to achieve it. Steps of testing: ./qemu-system-x86_64 -enable-kvm -m 4096 -smp 4 -name redhat6.2 -drive \ file=/home/redhat6.2.img,if=none,id=drive-ide0-0-0 \ -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ -drive file=/home/RH-DVD1.iso,if=none,id=drive-ide0-0-1 \ -device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1,bootindex=4 \ -vnc 0.0.0.0:10 -netdev type=user,id=net0 \ -device virtio-net-pci,netdev=net0,bootindex=3,id=nic1 \ -netdev type=user,id=net1 -device e1000,netdev=net1,bootindex=2,id=nic2 \ -drive file=/home/virtio-disk.vfd,if=none,id=drive-fdc0-0-0,format=raw \ -device isa-fdc,driveA=drive-fdc0-0-0,id=floppy1,bootindexA=5 -monitor stdio QEMU 2.0.93 monitor - type 'help' for more information (qemu) info bootindex id bootindex suffix floppy15 /floppy@0 ide0-0-1 4 /disk@1 nic1 3 /ethernet-phy@0 nic2 2 /ethernet-phy@0 ide0-0-0 1 /disk@0 (qemu) set-bootindex ide0-0-1 1 The bootindex 1 has already been used (qemu) set-bootindex ide0-0-1 6 /disk@1 (qemu) set-bootindex ide0-0-1 0 (qemu) system_reset (qemu) set-bootindex ide0-0-1 1 The bootindex 1 has already been used (qemu) set-bootindex nic1 0 The bootindex 0 has already been used (qemu) set-bootindex ide0-0-1 -1 (qemu) set-bootindex nic1 0 (qemu) info bootindex id bootindex suffix floppy15 /floppy@0 nic2 2 /ethernet-phy@0 ide0-0-0 1 /disk@0 nic1 0 /ethernet-phy@0 (qemu) system_reset (qemu) Changes since v4: - using error_setg() instead of qerror_report() in patch 1/8. - call del_boot_device_path() from device_finalize() instead of placing it into each individual device in patch 4/8. Changes since v3: - rework del_* and modify_* function, because of virtio devices' specialation. For example, virtio-net's id is NULL, and its parent virtio-net-pci's id was assigned. Though the global fw_boot_order stored the virtio-net device. - call dell_boot_device_path in each individual device avoiding waste resouce. - introduce qmp query-bootindex command - introcude hmp info bootindex command - Fixes by Eric's reviewing comments, thanks. Changes since v2: *address Gerd's reviewing suggestion: - use the old entry's suffix, if the caller do not pass it in. - call del_boot_device_path() from device_finalize() instead of placing it into each individual device. Thanks Gerd. Changes since v1: *rework by Gerd's suggestion: - split modify and del fw_boot_order for single function. - change modify bootindex's realization which simply lookup the device and modify the bootindex. if the new bootindex has already used by another device just throw an error. - change to del_boot_device_path(DeviceState *dev) and simply delete all entries belonging to the device. Gonglei (8): bootindex: add modify_boot_device_path function bootindex: add del_boot_device_path function fw_cfg: add fw_cfg_machine_reset function bootindex: delete bootindex when device is removed qmp: add set-bootindex command qemu-monitor: HMP set-bootindex wrapper qmp: add query-bootindex command qemu-monitor: add HMP info-bootindex command hmp-commands.hx | 17 +++ hmp.c | 33 + hmp.h | 2 + hw/core/qdev.c| 4 ++ hw/nvram/fw_cfg.c | 54 ++--- include/hw/nvram/fw_cfg.h | 2 + include/sysemu/sysemu.h | 3 ++ monitor.c | 7 +++ qapi-schema.json | 46 ++ qmp-commands.hx | 66 ++ qmp.c | 17 +++ vl.c | 117 ++ 12 files changed, 361 insertions(+), 7 deletions(-) -- 1.7.12.4
Re: [Qemu-devel] [PATCH v2 03/10] target-arm: Support save/load for 64 bit CPUs
On 10 July 2014 16:50, Alex Bennée alex.ben...@linaro.org wrote:
This enables the saving and restoring of machine state by including the
current program state (*psr) and xregs. The save_state_to_spsr hides the
details of if the processor is in 32 or 64 bit mode at the time.
Signed-off-by: Alex Bennée alex.ben...@linaro.org
---
v2 (ajb)
- use common state save functions
- re-base to latest origin/master
- clean up commented out code
diff --git a/target-arm/machine.c b/target-arm/machine.c
index 3bcc7cc..759610c 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -120,30 +120,27 @@ static const VMStateDescription vmstate_thumb2ee = {
}
};
-static int get_cpsr(QEMUFile *f, void *opaque, size_t size)
+static int get_psr(QEMUFile *f, void *opaque, size_t size)
{
ARMCPU *cpu = opaque;
CPUARMState *env = cpu-env;
uint32_t val = qemu_get_be32(f);
-/* Avoid mode switch when restoring CPSR */
-env-uncached_cpsr = val CPSR_M;
-cpsr_write(env, val, 0x);
+restore_state_from_spsr(env, val);
return 0;
}
-static void put_cpsr(QEMUFile *f, void *opaque, size_t size)
+static void put_psr(QEMUFile *f, void *opaque, size_t size)
{
ARMCPU *cpu = opaque;
CPUARMState *env = cpu-env;
-
-qemu_put_be32(f, cpsr_read(env));
+qemu_put_be32(f, save_state_to_spsr(env));
}
-static const VMStateInfo vmstate_cpsr = {
+static const VMStateInfo vmstate_psr = {
.name = cpsr,
-.get = get_cpsr,
-.put = put_cpsr,
+.get = get_psr,
+.put = put_psr,
};
static void cpu_pre_save(void *opaque)
@@ -218,17 +215,19 @@ static int cpu_post_load(void *opaque, int version_id)
const VMStateDescription vmstate_arm_cpu = {
.name = cpu,
-.version_id = 20,
-.minimum_version_id = 20,
+.version_id = 21,
+.minimum_version_id = 21,
.pre_save = cpu_pre_save,
.post_load = cpu_post_load,
.fields = (VMStateField[]) {
VMSTATE_UINT32_ARRAY(env.regs, ARMCPU, 16),
+VMSTATE_UINT64_ARRAY(env.xregs, ARMCPU, 32),
+VMSTATE_UINT64(env.pc, ARMCPU),
{
-.name = cpsr,
+.name = psr,
Why do we rename cpsr to psr here but not in the
vmstate_psr itself above? (Personally I would call this
pstate or just leave it as cpsr, but naming here isn't
a big deal I think.)
.version_id = 0,
.size = sizeof(uint32_t),
-.info = vmstate_cpsr,
+.info = vmstate_psr,
.flags = VMS_SINGLE,
.offset = 0,
},
thanks
-- PMM
[Qemu-devel] [PATCH v5 7/8] qmp: add query-bootindex command
From: Gonglei arei.gong...@huawei.com
Adds query-bootindex QMP command.
Example QMP command:
- { execute: query-bootindex}
- {
return:[
{
id:ide0-0-0,
bootindex:1,
suffix:/disk@0
},
{
id:nic1,
bootindex:2,
suffix:/ethernet-phy@0
}
]
}
Signed-off-by: Gonglei arei.gong...@huawei.com
Signed-off-by: Chenliang chenlian...@huawei.com
---
qapi-schema.json | 29 +
qmp-commands.hx | 41 +
vl.c | 31 +++
3 files changed, 101 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json
index 30bd6ad..680cbc5 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1704,6 +1704,33 @@
{ 'command': 'device_del', 'data': {'id': 'str'} }
##
+# @BootindexInfo:
+#
+# Information about devcie's bootindex.
+#
+# @id: the name of a device owning the bootindex
+#
+# @bootindex: the bootindex number
+#
+# @suffix: the suffix a device's bootindex
+#
+# Since: 2.2
+##
+{ 'type': 'BootindexInfo',
+ 'data': {'id': 'str', 'bootindex': 'int', 'suffix': 'str'} }
+
+##
+# @query-bootindex:
+#
+# Returns information about bootindex
+#
+# Returns: a list of @BootindexInfo for existing device
+#
+# Since: 2.2
+##
+{ 'command': 'query-bootindex', 'returns': ['BootindexInfo'] }
+
+##
# @set-bootindex:
#
# set bootindex of a device
@@ -1715,6 +1742,8 @@
# Returns: Nothing on success
# If @id is not a valid device, DeviceNotFound
#
+# Note: suffix can be gotten by query-bootindex command
+#
# Since: 2.2
##
{ 'command': 'set-bootindex',
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 19cc3b8..6ab9325 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -337,6 +337,47 @@ EQMP
},
SQMP
+query-bootindex
+---
+
+Show VM bootindex information.
+
+The returned value is a json-array of all device configured
+bootindex property. Each bootindex is represented by a json-object.
+
+The bootindex json-object contains the following:
+
+- id: the name of a device owning the bootindex (json-string)
+- bootindex: the bootindex number (json-int)
+- suffix: the suffix a device's bootindex (json-string)
+
+Example:
+
+- { execute: query-bootindex }
+- {
+ return:[
+ {
+id:ide0-0-0,
+bootindex:1,
+suffix:/disk@0
+ },
+ {
+id:nic1,
+bootindex:2,
+suffix:/ethernet-phy@0
+ }
+ ]
+ }
+
+EQMP
+
+{
+.name = query-bootindex,
+.args_type = ,
+.mhandler.cmd_new = qmp_marshal_input_query_bootindex,
+},
+
+SQMP
set-bootindex
-
diff --git a/vl.c b/vl.c
index 2424135..b1d8896 100644
--- a/vl.c
+++ b/vl.c
@@ -1219,6 +1219,37 @@ static void restore_boot_order(void *opaque)
g_free(normal_boot_order);
}
+BootindexInfoList *qmp_query_bootindex(Error **errp)
+{
+BootindexInfoList *booindex_list = NULL;
+BootindexInfoList *info;
+FWBootEntry *i;
+
+QTAILQ_FOREACH(i, fw_boot_order, link) {
+info = g_new0(BootindexInfoList, 1);
+info-value = g_new0(BootindexInfo, 1);
+
+if (i-dev-id) {
+info-value-id = g_strdup(i-dev-id);
+} else {
+/* For virtio devices, we should get id from they parent */
+if (i-dev-parent_bus) {
+DeviceState *dev = i-dev-parent_bus-parent;
+info-value-id = g_strdup(dev-id);
+} else {
+info-value-id = g_strdup();
+}
+}
+info-value-bootindex = i-bootindex;
+info-value-suffix = g_strdup(i-suffix);
+
+info-next = booindex_list;
+booindex_list = info;
+}
+
+return booindex_list;
+}
+
void add_boot_device_path(int32_t bootindex, DeviceState *dev,
const char *suffix)
{
--
1.7.12.4
Re: [Qemu-devel] [PATCH v5 0/8] modify boot order of guest, and take effect after rebooting
Hi, I' am so sorry for missing cc'ed Eduardo. Best regards, -Gonglei -Original Message- From: Gonglei (Arei) Sent: Monday, August 04, 2014 8:46 PM To: qemu-devel@nongnu.org Subject: [PATCH v5 0/8] modify boot order of guest, and take effect after rebooting From: Gonglei arei.gong...@huawei.com Sometimes, we want to modify boot order of a guest, but no need to shutdown it. We can call dynamic changing bootindex of a guest, which can be assured taking effect just after the guest rebooting. For example, in P2V scene, we boot a guest and then attach a new system disk, for copying some thing. We want to assign the new disk as the booting disk, which means its bootindex=1. Different nics can be assigen different bootindex dynamically also make sense. The patchsets add one qmp interface, and add an fw_cfg_machine_reset() to achieve it. Steps of testing: ./qemu-system-x86_64 -enable-kvm -m 4096 -smp 4 -name redhat6.2 -drive \ file=/home/redhat6.2.img,if=none,id=drive-ide0-0-0 \ -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ -drive file=/home/RH-DVD1.iso,if=none,id=drive-ide0-0-1 \ -device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1,bootindex=4 \ -vnc 0.0.0.0:10 -netdev type=user,id=net0 \ -device virtio-net-pci,netdev=net0,bootindex=3,id=nic1 \ -netdev type=user,id=net1 -device e1000,netdev=net1,bootindex=2,id=nic2 \ -drive file=/home/virtio-disk.vfd,if=none,id=drive-fdc0-0-0,format=raw \ -device isa-fdc,driveA=drive-fdc0-0-0,id=floppy1,bootindexA=5 -monitor stdio QEMU 2.0.93 monitor - type 'help' for more information (qemu) info bootindex id bootindex suffix floppy15 /floppy@0 ide0-0-1 4 /disk@1 nic1 3 /ethernet-phy@0 nic2 2 /ethernet-phy@0 ide0-0-0 1 /disk@0 (qemu) set-bootindex ide0-0-1 1 The bootindex 1 has already been used (qemu) set-bootindex ide0-0-1 6 /disk@1 (qemu) set-bootindex ide0-0-1 0 (qemu) system_reset (qemu) set-bootindex ide0-0-1 1 The bootindex 1 has already been used (qemu) set-bootindex nic1 0 The bootindex 0 has already been used (qemu) set-bootindex ide0-0-1 -1 (qemu) set-bootindex nic1 0 (qemu) info bootindex id bootindex suffix floppy15 /floppy@0 nic2 2 /ethernet-phy@0 ide0-0-0 1 /disk@0 nic1 0 /ethernet-phy@0 (qemu) system_reset (qemu) Changes since v4: - using error_setg() instead of qerror_report() in patch 1/8. - call del_boot_device_path() from device_finalize() instead of placing it into each individual device in patch 4/8. Changes since v3: - rework del_* and modify_* function, because of virtio devices' specialation. For example, virtio-net's id is NULL, and its parent virtio-net-pci's id was assigned. Though the global fw_boot_order stored the virtio-net device. - call dell_boot_device_path in each individual device avoiding waste resouce. - introduce qmp query-bootindex command - introcude hmp info bootindex command - Fixes by Eric's reviewing comments, thanks. Changes since v2: *address Gerd's reviewing suggestion: - use the old entry's suffix, if the caller do not pass it in. - call del_boot_device_path() from device_finalize() instead of placing it into each individual device. Thanks Gerd. Changes since v1: *rework by Gerd's suggestion: - split modify and del fw_boot_order for single function. - change modify bootindex's realization which simply lookup the device and modify the bootindex. if the new bootindex has already used by another device just throw an error. - change to del_boot_device_path(DeviceState *dev) and simply delete all entries belonging to the device. Gonglei (8): bootindex: add modify_boot_device_path function bootindex: add del_boot_device_path function fw_cfg: add fw_cfg_machine_reset function bootindex: delete bootindex when device is removed qmp: add set-bootindex command qemu-monitor: HMP set-bootindex wrapper qmp: add query-bootindex command qemu-monitor: add HMP info-bootindex command hmp-commands.hx | 17 +++ hmp.c | 33 + hmp.h | 2 + hw/core/qdev.c| 4 ++ hw/nvram/fw_cfg.c | 54 ++--- include/hw/nvram/fw_cfg.h | 2 + include/sysemu/sysemu.h | 3 ++ monitor.c | 7 +++ qapi-schema.json | 46 ++ qmp-commands.hx | 66 ++ qmp.c | 17 +++ vl.c | 117 ++ 12 files changed, 361 insertions(+), 7 deletions(-) -- 1.7.12.4
[Qemu-devel] [PATCH v5 8/8] qemu-monitor: add HMP info-bootindex command
From: Gonglei arei.gong...@huawei.com
Add HMP info-bootindex command to getting
devcie's bootindex via monitor.
Signed-off-by: Gonglei arei.gong...@huawei.com
Signed-off-by: Chenliang chenlian...@huawei.com
---
hmp-commands.hx | 2 ++
hmp.c | 20
hmp.h | 1 +
monitor.c | 7 +++
4 files changed, 30 insertions(+)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 31ef24e..bc1b982 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1795,6 +1795,8 @@ show qdev device model list
show roms
@item info tpm
show the TPM device
+@item info bootindex
+show the current VM bootindex information
@end table
ETEXI
diff --git a/hmp.c b/hmp.c
index 95f7eeb..1688e02 100644
--- a/hmp.c
+++ b/hmp.c
@@ -725,6 +725,26 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
qapi_free_TPMInfoList(info_list);
}
+void hmp_info_bootindex(Monitor *mon, const QDict *qdict)
+{
+BootindexInfoList *bootindex_list, *info;
+
+bootindex_list = qmp_query_bootindex(NULL);
+if (!bootindex_list) {
+monitor_printf(mon, No bootindex was configured\n);
+return;
+}
+
+monitor_printf(mon, id \t bootindex \t suffix\n);
+for (info = bootindex_list; info; info = info-next) {
+monitor_printf(mon, \%s\\t %PRId64\t\%s\\n,
+ info-value-id, info-value-bootindex,
+ info-value-suffix);
+}
+
+qapi_free_BootindexInfoList(bootindex_list);
+}
+
void hmp_quit(Monitor *mon, const QDict *qdict)
{
monitor_suspend(mon);
diff --git a/hmp.h b/hmp.h
index eb2641a..5899537 100644
--- a/hmp.h
+++ b/hmp.h
@@ -38,6 +38,7 @@ void hmp_info_balloon(Monitor *mon, const QDict *qdict);
void hmp_info_pci(Monitor *mon, const QDict *qdict);
void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
void hmp_info_tpm(Monitor *mon, const QDict *qdict);
+void hmp_info_bootindex(Monitor *mon, const QDict *qdict);
void hmp_quit(Monitor *mon, const QDict *qdict);
void hmp_stop(Monitor *mon, const QDict *qdict);
void hmp_system_reset(Monitor *mon, const QDict *qdict);
diff --git a/monitor.c b/monitor.c
index 5bc70a6..8158ddb 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2918,6 +2918,13 @@ static mon_cmd_t info_cmds[] = {
.mhandler.cmd = hmp_info_memdev,
},
{
+.name = bootindex,
+.args_type = ,
+.params = ,
+.help = show the current VM bootindex information,
+.mhandler.cmd = hmp_info_bootindex,
+},
+{
.name = NULL,
},
};
--
1.7.12.4
[Qemu-devel] [PATCH v5 6/8] qemu-monitor: HMP set-bootindex wrapper
From: Gonglei arei.gong...@huawei.com
Add HMP set-bootindex wrapper to allow setting
devcie's bootindex via monitor.
Signed-off-by: Gonglei arei.gong...@huawei.com
Signed-off-by: Chenliang chenlian...@huawei.com
---
hmp-commands.hx | 15 +++
hmp.c | 13 +
hmp.h | 1 +
3 files changed, 29 insertions(+)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index d0943b1..31ef24e 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -688,6 +688,21 @@ Remove device @var{id}.
ETEXI
{
+.name = set-bootindex,
+.args_type = id:s,bootindex:l,suffix:s?,
+.params = device bootindex [suffix],
+.help = set bootindex of a device(e.g. set-bootindex disk0 1
'/disk@0'),
+.mhandler.cmd = hmp_set_bootindex,
+},
+
+STEXI
+@item set-bootindex @var{id} @var{bootindex}
+@findex set-bootindex
+
+Set bootindex of a device.
+ETEXI
+
+{
.name = cpu,
.args_type = index:i,
.params = index,
diff --git a/hmp.c b/hmp.c
index 4d1838e..95f7eeb 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1714,3 +1714,16 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
monitor_printf(mon, \n);
}
+
+void hmp_set_bootindex(Monitor *mon, const QDict *qdict)
+{
+Error *err = NULL;
+
+const char *id = qdict_get_str(qdict, id);
+int64_t bootindex = qdict_get_int(qdict, bootindex);
+bool has_suffix = qdict_haskey(qdict, suffix);
+const char *suffix = qdict_get_try_str(qdict, suffix);
+
+qmp_set_bootindex(id, bootindex, has_suffix, suffix, err);
+hmp_handle_error(mon, err);
+}
diff --git a/hmp.h b/hmp.h
index 4fd3c4a..eb2641a 100644
--- a/hmp.h
+++ b/hmp.h
@@ -94,6 +94,7 @@ void hmp_cpu_add(Monitor *mon, const QDict *qdict);
void hmp_object_add(Monitor *mon, const QDict *qdict);
void hmp_object_del(Monitor *mon, const QDict *qdict);
void hmp_info_memdev(Monitor *mon, const QDict *qdict);
+void hmp_set_bootindex(Monitor *mon, const QDict *qdict);
void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
--
1.7.12.4
