Re: [Qemu-devel] [PATCH V3 0/7] export internal snapshot by qemu-nbd
于 2013/9/26 8:16, Wenchao Xia 写道: This series allow user to read internal snapshot's contents without qemu-img convert. V2: Address Stefan's comments: 02: add 'fall through' comments in the case statement. 03: add doc about the difference of internal snapshot and backing chain snapshot, which is used in previous '--snapshot' parameter. Other: 01,04: rebased on upstream with conflict resolved. v3: Address Paolo's comments: 02: add parameter -l snapshot_id_or_name, rename options snapshot-load to load-snapshot, use QemuOpts. 03: rename snapshot-load to load-snapshot. 04: related change to test both -l and -L case. 05-07: add similar parameter for qemu-img convert. Other: 01: foldered original snapshot logic into function bdrv_snapshot_load_tmp_by_id_or_name(), since multiple caller present in this version. Refined error message from , reason: %s to : %s. 02: Refined error message from , reason: %s to : %s. 03: Rename PARAM to SNAPSHOT_PARAM. Wenchao Xia (7): 1 snapshot: distinguish id and name in load_tmp 2 qemu-nbd: support internal snapshot export 3 qemu-nbd: add doc for internal snapshot export 4 qemu-iotests: add 058 internal snapshot export with qemu-nbd case 5 qemu-img: add -L for snapshot in convert 6 qemu-img: add doc for param -L in convert 7 qemu-iotests: add test for snapshot in qemu-img convert block/qcow2-snapshot.c | 16 +- block/qcow2.h |5 ++- block/snapshot.c | 76 +++- include/block/block_int.h |4 +- include/block/snapshot.h | 13 - qemu-img-cmds.hx |2 +- qemu-img.c | 32 +--- qemu-img.texi |7 ++- qemu-nbd.c | 46 - qemu-nbd.texi | 11 - tests/qemu-iotests/058 | 121 tests/qemu-iotests/058.out | 44 tests/qemu-iotests/check |1 + tests/qemu-iotests/group |1 + 14 files changed, 359 insertions(+), 20 deletions(-) create mode 100755 tests/qemu-iotests/058 create mode 100644 tests/qemu-iotests/058.out Any commenst for this simple series? :)
Re: [Qemu-devel] help me with qemu-img theory(snapshot and backing file)
On Mon, 09/30 12:48, yue-kvm wrote: hi,all if there any documents about snapshot and backing file's theory. as we know vmware has composor which can upgrade images(top of backing chain ) by upgrading backing file What does upgrade images mean? Thanks, Fam
Re: [Qemu-devel] [Qemu-trivial] [PATCH] pci-ohci: Add missing 'break' in ohci_service_td
Hi, While the actual interesting change (adding break) looks correct, and the whole thing is trivial indeed, this area has a maintainer, -- Cc'ing Gerd for this. If he's okay I'll pick it up. Patch is fine. Acked-by: Gerd Hoffmann kra...@gmail.com cheers, Gerd
[Qemu-devel] [PATCH] qcow2: Correct endianness in overlap check
If an inactive L1 table is loaded from disk, its entries are in big endian and have to be converted to host byte order before using them. Signed-off-by: Max Reitz mre...@redhat.com --- block/qcow2-refcount.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index d2b7064..364eeba 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1733,8 +1733,8 @@ int qcow2_check_metadata_overlap(BlockDriverState *bs, int chk, int64_t offset, } for (j = 0; j l1_sz; j++) { -if ((l1[j] L1E_OFFSET_MASK) -overlaps_with(l1[j] L1E_OFFSET_MASK, s-cluster_size)) { +uint64_t l2_ofs = be64_to_cpu(l1[j]) L1E_OFFSET_MASK; +if (l2_ofs overlaps_with(l2_ofs, s-cluster_size)) { g_free(l1); return QCOW2_OL_INACTIVE_L2; } -- 1.8.3.1
[Qemu-devel] [PATCH] qcow2: CHECK_OFLAG_COPIED is obsolete
CHECK_OFLAG_COPIED as a parameter to check_refcounts_l1 and check_refcounts_l2 is obselete now, since the OFLAG_COPIED consistency check is actually no longer performed by these functions (but by check_oflag_copied). Signed-off-by: Max Reitz mre...@redhat.com --- block/qcow2-refcount.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index d2b7064..c0b4184 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1034,7 +1034,6 @@ static void inc_refcounts(BlockDriverState *bs, /* Flags for check_refcounts_l1() and check_refcounts_l2() */ enum { -CHECK_OFLAG_COPIED = 0x1, /* check QCOW_OFLAG_COPIED matches refcount */ CHECK_FRAG_INFO = 0x2, /* update BlockFragInfo counters */ }; @@ -1481,8 +1480,7 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res, /* current L1 table */ ret = check_refcounts_l1(bs, res, refcount_table, nb_clusters, - s-l1_table_offset, s-l1_size, - CHECK_OFLAG_COPIED | CHECK_FRAG_INFO); + s-l1_table_offset, s-l1_size, CHECK_FRAG_INFO); if (ret 0) { goto fail; } -- 1.8.3.1
Re: [Qemu-devel] drive-backup locks VM if target has issues?
Il 30/09/2013 00:46, Wolfgang Richter ha scritto: I wanted to explore overhead with the new drive-backup command and I noticed if I set the target to something like '/dev/null' the guest VM starts having IO errors and loses write access to its root file system. Here is the qmp-shell command I'm using: drive-backup sync=none device=virtio0 target=/dev/null format=raw mode=existing I have a guest running with a single virtio root disk (ext4, Ubuntu guest). After that command, the guest sees write errors to its root block device (virtio0). All writes to the drive-backup source have to first copy the pre-write data to the target. Thus, drive-backup usually works best if you are using werror=stop on the source. That said, I would have expected the job to be cancelled instead. Looks like there are bugs in the handling of on_target_error. I didn't trace syscalls or dig deeper yet, but was wondering if you had an idea on why '/dev/null' as a target in a block job would cause the origin device to lockup/fail? My overall goal is to drop the extra write traffic as early as possible to measure overhead of the drive-backup command in a few different scenarios, thus I was hoping /dev/null would help here. I think you need a null backend instead that drops writes at the QEMU level. Perhaps /dev/zero helps too. Paolo
Re: [Qemu-devel] [PATCH v6 00/26] qemu: generate acpi tables for the guest
On So, 2013-09-29 at 13:58 +0300, Michael S. Tsirkin wrote: This code can also be found here: git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git acpi Reviewed-and-tested-by: Gerd Hoffmann kra...@redhat.com cheers, Gerd
Re: [Qemu-devel] [Qemu-trivial] [PATCH] block: Remove unused assignment (fixes warning from clang)
Am 29.09.2013 um 22:15 hat Stefan Weil geschrieben: Am 29.09.2013 21:44, schrieb Michael Tokarev: 28.09.2013 13:55, Stefan Weil wrote: [...] diff --git a/blockdev.c b/blockdev.c index 8aa66a9..8c83f6f 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1926,7 +1926,6 @@ void qmp_drive_mirror(const char *device, const char *target, } else { switch (mode) { case NEW_IMAGE_MODE_EXISTING: -ret = 0; break; While this one is obviously unused assignment, there's on more usage of `ret' variable in this function, -- it is to store the return value from bdrv_open(): ret = bdrv_open(target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv, local_err); if (ret 0) {... What's the rule about converting that into if() ? Thanks, /mjt Is there a rule for cases like that? This pattern is very common in QEMU code (several occurrences in blockdev.c). Should we eliminate the 'ret' variable? I don't think it's worth the effort. And actually I think removing it would make the code worse (less readable). Kevin
Re: [Qemu-devel] [Qemu-trivial] [PATCH] pci-ohci: Add missing 'break' in ohci_service_td
30.09.2013 10:32, Gerd Hoffmann wrote: Hi, While the actual interesting change (adding break) looks correct, and the whole thing is trivial indeed, this area has a maintainer, -- Cc'ing Gerd for this. If he's okay I'll pick it up. Patch is fine. No, I didn't ask whenever the patch is fine, I wanted to ensure you wont blame me for accepting a trivial patch for an area which has an active maintainer... ;) Thanks, queued up for trivial-patches! /mjt
[Qemu-devel] Minimal Qemu build for Plan9
Hi, I am a graduate student from CMU and currently working on trying to port Qemu on plan9. I was trying to initially build a minimal Qemu with the least set of devices that are necessary to support a machine, however I noticed that I couldn't understand some of the dependencies such as requiring an audio driver during initialization. I am currently trying to port just the i386-softmmu version of Qemu to plan9 to keep things simple, here I noticed that pc_init1() initializes a host of devices on the PCI bus like the NIC card etc and was wondering if there is a bare minimum set of devices that Qemu needs to be built with ? Additionally if someone could point me to a to-do list of things to look out for while porting Qemu on a new platform (new OS/hardware), I would be extremely grateful. Thanks Regards Ashish
Re: [Qemu-devel] [PATCH v5 2/5] hpet: entitle more irq pins for hpet
On Sun, Sep 29, 2013 at 12:15 PM, Michael S. Tsirkin m...@redhat.com wrote: On Sun, Sep 29, 2013 at 11:49:41AM +0800, liu ping fan wrote: On Sun, Sep 29, 2013 at 3:56 AM, Michael S. Tsirkin m...@redhat.com wrote: On Thu, Sep 12, 2013 at 11:25:15AM +0800, Liu Ping Fan wrote: On PC, IRQ2/8 can be reserved for hpet timer 0/1. And pin 16~23 of ioapic can be dynamically assigned to hpet as guest chooses. (Will enable them after introducing pc 1.6 compat) Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com --- hw/timer/hpet.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c index 8429eb3..46903b9 100644 --- a/hw/timer/hpet.c +++ b/hw/timer/hpet.c @@ -25,6 +25,7 @@ */ #include hw/hw.h +#include hw/boards.h #include hw/i386/pc.h #include ui/console.h #include qemu/timer.h @@ -42,6 +43,12 @@ #define HPET_MSI_SUPPORT0 +/* For bug compat, using only IRQ2. Soon it will be fixed as + * 0xff0104ULL, i.e using IRQ16~23, IRQ8 and IRQ2 So users are expected to stick a bitmask of legal pins here? I think that's a bit too much rope to give to users. Don't you think? Sorry, not understand your meaning exactly. But the scene will be: guest kernel polls the ability bitmask, and pick up one pin which is not occupied or can be shared with the level-trigger and low-active. So is it rope? I merely say that it's better to make this a bool or bit property. UINT32 is too much flexibility imho. The interrupt capability is export to guest by register Tn_INT_ROUTE_CAP[63:32]. So it is useless to make them as a bit property. Do you think so? Regards Pingfan config register in hpet con Thanks and regards, Pingfan after + * introducing pc-1.6 compat. + */ +#define HPET_TN_INT_CAP_DEFAULT 0x4ULL + #define TYPE_HPET hpet #define HPET(obj) OBJECT_CHECK(HPETState, (obj), TYPE_HPET) @@ -73,6 +80,7 @@ typedef struct HPETState { uint8_t rtc_irq_level; qemu_irq pit_enabled; uint8_t num_timers; +uint32_t intcap; HPETTimer timer[HPET_MAX_TIMERS]; /* Memory-mapped, software visible registers */ @@ -663,8 +671,8 @@ static void hpet_reset(DeviceState *d) if (s-flags (1 HPET_MSI_SUPPORT)) { timer-config |= HPET_TN_FSB_CAP; } -/* advertise availability of ioapic inti2 */ -timer-config |= 0x0004ULL 32; +/* advertise availability of ioapic int */ +timer-config |= (uint64_t)s-intcap 32; timer-period = 0ULL; timer-wrap_flag = 0; } @@ -753,6 +761,7 @@ static void hpet_realize(DeviceState *dev, Error **errp) static Property hpet_device_properties[] = { DEFINE_PROP_UINT8(timers, HPETState, num_timers, HPET_MIN_TIMERS), DEFINE_PROP_BIT(msi, HPETState, flags, HPET_MSI_SUPPORT, false), +DEFINE_PROP_UINT32(intcap, HPETState, intcap, HPET_TN_INT_CAP_DEFAULT), DEFINE_PROP_END_OF_LIST(), }; -- 1.8.1.4
Re: [Qemu-devel] [PATCH v5 1/5] hpet: inverse polarity when pin above ISA_NUM_IRQS
On Sun, Sep 29, 2013 at 12:20 PM, Michael S. Tsirkin m...@redhat.com wrote: On Sun, Sep 29, 2013 at 11:25:24AM +0800, liu ping fan wrote: On Sun, Sep 29, 2013 at 3:52 AM, Michael S. Tsirkin m...@redhat.com wrote: On Thu, Sep 12, 2013 at 11:25:14AM +0800, Liu Ping Fan wrote: According to hpet spec, hpet irq is high active. But according to ICH spec, there is inversion before the input of ioapic. So the OS will expect low active on this IRQ line. (And this is observed on bare metal). How does one test this on bare metal? If changing the func hpet_timer_set_irq() in linux's hpet driver, from acpi_register_gsi(NULL, irq, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW); to ACPI_ACTIVE_HIGH, then run hpet_example.c on bare metal, the modified kernel will complain about spurious irq and disable the irq line. ok that's useful info for the changelog. Will document them. We fold the emulation of this inversion inside the hpet logic. Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com Doesn't this affect cross-version migration? E.g. imagine that you migrate between systems with/without this fix. No. the changing only affect route = ISA_NUM_IRQS, For linux guest, it use IRQ2/8 for hpet0/hpet1 which is reserved by kernel. It work without this fix (I think windows is the same). But the hpet_example.c(in linux) can not work without this fix. So no such run-time instance before this bug fix. Regards, Pingfan aha so the argument is it's already too broken to even mostly work, we don't need to worry about migrating it to/from old qemu. Yes :) Thanks, Pingfan
Re: [Qemu-devel] [PATCH] qcow2: Correct endianness in overlap check
Am 30.09.2013 um 08:59 hat Max Reitz geschrieben: If an inactive L1 table is loaded from disk, its entries are in big endian and have to be converted to host byte order before using them. Signed-off-by: Max Reitz mre...@redhat.com Reviewed-by: Kevin Wolf kw...@redhat.com Sounds like there's yet a test case missing? (But it requires the runtime options for enabling these checks, obviously) Kevin
Re: [Qemu-devel] [PATCH] qcow2: CHECK_OFLAG_COPIED is obsolete
Am 30.09.2013 um 09:21 hat Max Reitz geschrieben: CHECK_OFLAG_COPIED as a parameter to check_refcounts_l1 and check_refcounts_l2 is obselete now, since the OFLAG_COPIED consistency check is actually no longer performed by these functions (but by check_oflag_copied). Signed-off-by: Max Reitz mre...@redhat.com Reviewed-by: Kevin Wolf kw...@redhat.com
Re: [Qemu-devel] [PATCH] qcow2: Correct endianness in overlap check
On 2013-09-30 10:06, Kevin Wolf wrote: Am 30.09.2013 um 08:59 hat Max Reitz geschrieben: If an inactive L1 table is loaded from disk, its entries are in big endian and have to be converted to host byte order before using them. Signed-off-by: Max Reitz mre...@redhat.com Reviewed-by: Kevin Wolf kw...@redhat.com Sounds like there's yet a test case missing? (But it requires the runtime options for enabling these checks, obviously) Yes to both. For now, I just couldn't write such a test case. Max
Re: [Qemu-devel] [PATCH 0/3] hw: set irq without selecting INTx pin
On Sun, 2013-09-29 at 18:06 +0300, Michael S. Tsirkin wrote: On Sun, Sep 29, 2013 at 05:40:54PM +0300, Marcel Apfelbaum wrote: Interrupt pin is selected and saved into PCI_INTERRUPT_PIN register during device initialization. Devices should not call directly qemu_set_irq and specify the INTx pin. Replaced the call to qemu_set_irq with a new wrapper pci_set_irq which triggers the irq based on PCI_INTERRUPT_PIN. Looks good overall. As a next step, can we make pci_set_irq non-inline and make it call pci_irq_handler directly, and get rid of the irq field? What irq field? Thanks, Marcel This way we know no one is using it directly ... Marcel Apfelbaum (3): hw/pci: set irq without selecting INTx pin hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init hw: assert/deassert interrupts using pci_set_irq wrapper hw/audio/ac97.c| 4 ++-- hw/audio/es1370.c | 2 +- hw/audio/intel-hda.c | 2 +- hw/char/serial-pci.c | 2 +- hw/char/tpci200.c | 4 ++-- hw/display/qxl.c | 2 +- hw/ide/cmd646.c| 2 +- hw/isa/vt82c686.c | 2 +- hw/misc/ivshmem.c | 2 +- hw/net/e1000.c | 2 +- hw/net/rtl8139.c | 2 +- hw/pci-bridge/pci_bridge_dev.c | 2 +- hw/pci/pci.c | 6 +++--- hw/pci/shpc.c | 2 +- hw/scsi/lsi53c895a.c | 2 +- hw/scsi/vmw_pvscsi.c | 2 +- hw/virtio/virtio-pci.c | 4 ++-- include/hw/pci/pci.h | 7 +++ 18 files changed, 29 insertions(+), 22 deletions(-) -- 1.8.3.1
Re: [Qemu-devel] qemu, numa: non-contiguous cpusets
On Sun, Sep 29, 2013 at 05:10:44PM +0200, Borislav Petkov wrote: Btw, while I got your attention, on a not-really related topic: how do we feel about adding support for specifying a non-contiguous set of cpus for a numa node in qemu with the -numa option? I.e., like this, for example: x86_64-softmmu/qemu-system-x86_64 -smp 8 -numa node,nodeid=0,cpus=0\;2\;4-5 -numa node,nodeid=1,cpus=1\;3\;6-7 The ';' needs to be escaped from the shell but I'm open for better suggestions. Use a ':' instead. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH] exec: Fix prototype of phys_mem_set_alloc and related functions
Stefan Weil s...@weilnetz.de writes: phys_mem_alloc and its assigned values qemu_anon_ram_alloc and legacy_s390_alloc must have identical argument lists. legacy_s390_alloc uses the size parameter to call mmap, so size_t is good enough for all of them. This patch fixes compiler errors on i686 Linux hosts: CCalpha-softmmu/exec.o exec.c:752:51: error: initialization from incompatible pointer type [-Werror] exec.c: In function 'qemu_ram_alloc_from_ptr': exec.c:1139:32: error: comparison of distinct pointer types lacks a cast [-Werror] exec.c: In function 'qemu_ram_remap': exec.c:1283:21: error: comparison of distinct pointer types lacks a cast [-Werror] Signed-off-by: Stefan Weil s...@weilnetz.de Reviewed-by: Markus Armbruster arm...@redhat.com
Re: [Qemu-devel] [PATCH v3 for 1.6 0/8] Guest memory allocation fixes cleanup
Stefan Weil s...@weilnetz.de writes: Am 31.07.2013 15:11, schrieb Markus Armbruster: All I wanted to do is exit(1) instead of abort() on guest memory allocation failure [07/08]. But that lead me into a minor #ifdef bog, and here's what I brought back. Enjoy! Testing: * Christian Borntraeger reports v1 works fine under LPAR (new S390 KVM, i.e. generic allocation) and as second guest under z/VM (old S390 KVM, i.e. legacy S390 allocation). Thanks for testing, and for catching a stupid mistake. v2 differs from v1 only in code that isn't reachable on S390. [...] Two patches from this series seem to cause compiler errors in latest QEMU ona 32 bit Ubuntu precise host: CCarm-softmmu/exec.o exec.c:752:51: error: initialization from incompatible pointer type [-Werror] exec.c: In function 'qemu_ram_alloc_from_ptr': exec.c:1139:32: error: comparison of distinct pointer types lacks a cast [-Werror] exec.c: In function 'qemu_ram_remap': exec.c:1283:21: error: comparison of distinct pointer types lacks a cast [-Werror] There is a mismatch of function prototypes (size_t - ram_addr_t): void *qemu_anon_ram_alloc(size_t size); static void *(*phys_mem_alloc)(ram_addr_t size) = qemu_anon_ram_alloc; It's strange that the buildbots don't complain. Indeed. Thanks for posting a fix!
Re: [Qemu-devel] [PATCH v2 2/7] smbios: Convert to QemuOpts
Michael S. Tsirkin m...@redhat.com writes: On Fri, Aug 16, 2013 at 03:18:29PM +0200, arm...@redhat.com wrote: From: Markus Armbruster arm...@redhat.com So that it can be set in config file for -readconfig. This tightens parsing of -smbios, and makes it more consistent with other options: unknown parameters are rejected, numbers with trailing junk are rejected, when a parameter is given multiple times, last rather than first wins, ... Signed-off-by: Markus Armbruster arm...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com [...] diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c index 0608aee..a113f8b 100644 --- a/hw/i386/smbios.c +++ b/hw/i386/smbios.c [...] @@ -225,17 +346,29 @@ void smbios_entry_add(const char *t) return; } -if (get_param_value(buf, sizeof(buf), type, t)) { -unsigned long type = strtoul(buf, NULL, 0); +val = qemu_opt_get(opts, type); +if (val) { +unsigned long type = strtoul(val, NULL, 0); + switch (type) { case 0: -smbios_build_type_0_fields(t); +qemu_opts_validate(opts, qemu_smbios_type0_opts, local_err); +if (local_err) { +error_report(%s, error_get_pretty(local_err)); +exit(1); +} +smbios_build_type_0_fields(opts); return; case 1: -smbios_build_type_1_fields(t); +qemu_opts_validate(opts, qemu_smbios_type1_opts, local_err); +if (local_err) { +error_report(%s, error_get_pretty(local_err)); +exit(1); +} +smbios_build_type_1_fields(opts); return; default: -error_report(Don't know how to build fields for SMBIOS type %ld, +error_report(Don't know how to build fields for SMBIOS type % PRIu64, type); exit(1); } This triggers a build failure: /scm/qemu/hw/i386/smbios.c: In function ‘smbios_entry_add’: /scm/qemu/hw/i386/smbios.c:382:26: error: format ‘%llu’ expects argument of type ‘long long unsigned int’, but argument 2 has type ‘long unsigned int’ [-Werror=format=] type); ^ It's a long value, why are you printing it with PRIu64? %ld seems right. Yup. I reverted just this chunk. Thanks for catching this. My compiler doesn't :( [...]
Re: [Qemu-devel] [Qemu-trivial] [PATCH v3 1/2] tests: Fix schema parser test for in-tree build
Michael Tokarev m...@tls.msk.ru writes: 24.09.2013 11:43, arm...@redhat.com wrote: From: Markus Armbruster arm...@redhat.com Commit 4f193e3 added the test, but screwed up in-tree builds (SRCDIR=.): the tests's output overwrites the expected output, and is thus compared to itself. [] .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y)) $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json - $(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts $(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py $^ $*.out 2$*.err; echo $$? $*.exit, TEST $*.out) -@diff -q $(SRC_PATH)/$*.out $*.out -@diff -q $(SRC_PATH)/$*.err $*.err -@diff -q $(SRC_PATH)/$*.exit $*.exit + $(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts $(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py $^ $*.test.out 2$*.test.err; echo $$? $*.test.exit, TEST $*.out) +@diff -q $(SRC_PATH)/$*.out $*.test.out +@diff -q $(SRC_PATH)/$*.err $*.test.err +@diff -q $(SRC_PATH)/$*.exit $*.test.exit Hmm. Maybe these new files should be cleaned up somehow by `make clean' ? Guess so. However, make clean doesn't clean *anything* in tests/ right now. Separate fix?
Re: [Qemu-devel] KVM call for agenda for 2013-10-01
On 09/24/2013 05:09 PM, Juan Quintela wrote: Hi Please, send any topic that you are interested in covering. Last week I forgot to send the call for topics. We still have a topic there. Thanks, Juan. Agenda so far: - Talk about qemu reverse executing (1st description was done this week) How to handle IO when we want to do reverse execution. How this relate to Kemari needs? Hi Juan, I'm sorry but I won't be able to attend to call tomorrow due to travel. Regards, Orit And to icount changes? Call details: 10:00 AM to 11:00 AM EDT Every two weeks If you need phone number details, contact me privately. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 0/3] hw: set irq without selecting INTx pin
On Mon, Sep 30, 2013 at 11:14:56AM +0300, Marcel Apfelbaum wrote: On Sun, 2013-09-29 at 18:06 +0300, Michael S. Tsirkin wrote: On Sun, Sep 29, 2013 at 05:40:54PM +0300, Marcel Apfelbaum wrote: Interrupt pin is selected and saved into PCI_INTERRUPT_PIN register during device initialization. Devices should not call directly qemu_set_irq and specify the INTx pin. Replaced the call to qemu_set_irq with a new wrapper pci_set_irq which triggers the irq based on PCI_INTERRUPT_PIN. Looks good overall. As a next step, can we make pci_set_irq non-inline and make it call pci_irq_handler directly, and get rid of the irq field? What irq field? /* IRQ objects for the INTA-INTD pins. */ qemu_irq *irq; Thanks, Marcel This way we know no one is using it directly ... Marcel Apfelbaum (3): hw/pci: set irq without selecting INTx pin hw/pci-bridge: set PCI_INTERRUPT_PIN register before shpc init hw: assert/deassert interrupts using pci_set_irq wrapper hw/audio/ac97.c| 4 ++-- hw/audio/es1370.c | 2 +- hw/audio/intel-hda.c | 2 +- hw/char/serial-pci.c | 2 +- hw/char/tpci200.c | 4 ++-- hw/display/qxl.c | 2 +- hw/ide/cmd646.c| 2 +- hw/isa/vt82c686.c | 2 +- hw/misc/ivshmem.c | 2 +- hw/net/e1000.c | 2 +- hw/net/rtl8139.c | 2 +- hw/pci-bridge/pci_bridge_dev.c | 2 +- hw/pci/pci.c | 6 +++--- hw/pci/shpc.c | 2 +- hw/scsi/lsi53c895a.c | 2 +- hw/scsi/vmw_pvscsi.c | 2 +- hw/virtio/virtio-pci.c | 4 ++-- include/hw/pci/pci.h | 7 +++ 18 files changed, 29 insertions(+), 22 deletions(-) -- 1.8.3.1
Re: [Qemu-devel] [PATCH 0/3] hw: set irq without selecting INTx pin
Il 30/09/2013 10:58, Michael S. Tsirkin ha scritto: As a next step, can we make pci_set_irq non-inline and make it call pci_irq_handler directly, and get rid of the irq field? What irq field? /* IRQ objects for the INTA-INTD pins. */ qemu_irq *irq; That's still used by devices that use common code for PCI and sysbus versions (e.g. USB OHCI and EHCI). Paolo
Re: [Qemu-devel] [PATCH v5 2/5] hpet: entitle more irq pins for hpet
On Mon, Sep 30, 2013 at 04:02:29PM +0800, liu ping fan wrote: On Sun, Sep 29, 2013 at 12:15 PM, Michael S. Tsirkin m...@redhat.com wrote: On Sun, Sep 29, 2013 at 11:49:41AM +0800, liu ping fan wrote: On Sun, Sep 29, 2013 at 3:56 AM, Michael S. Tsirkin m...@redhat.com wrote: On Thu, Sep 12, 2013 at 11:25:15AM +0800, Liu Ping Fan wrote: On PC, IRQ2/8 can be reserved for hpet timer 0/1. And pin 16~23 of ioapic can be dynamically assigned to hpet as guest chooses. (Will enable them after introducing pc 1.6 compat) Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com --- hw/timer/hpet.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c index 8429eb3..46903b9 100644 --- a/hw/timer/hpet.c +++ b/hw/timer/hpet.c @@ -25,6 +25,7 @@ */ #include hw/hw.h +#include hw/boards.h #include hw/i386/pc.h #include ui/console.h #include qemu/timer.h @@ -42,6 +43,12 @@ #define HPET_MSI_SUPPORT0 +/* For bug compat, using only IRQ2. Soon it will be fixed as + * 0xff0104ULL, i.e using IRQ16~23, IRQ8 and IRQ2 So users are expected to stick a bitmask of legal pins here? I think that's a bit too much rope to give to users. Don't you think? Sorry, not understand your meaning exactly. But the scene will be: guest kernel polls the ability bitmask, and pick up one pin which is not occupied or can be shared with the level-trigger and low-active. So is it rope? I merely say that it's better to make this a bool or bit property. UINT32 is too much flexibility imho. The interrupt capability is export to guest by register Tn_INT_ROUTE_CAP[63:32]. So it is useless to make them as a bit property. Do you think so? Regards Pingfan I think we merely need to support two modes: - qemu 1.6 and older compatible - compatible to actual hardware Why would we let users configure an arbitrary configuration which isn't compatible to either old qemu or real hardware? config register in hpet con Thanks and regards, Pingfan after + * introducing pc-1.6 compat. + */ +#define HPET_TN_INT_CAP_DEFAULT 0x4ULL + #define TYPE_HPET hpet #define HPET(obj) OBJECT_CHECK(HPETState, (obj), TYPE_HPET) @@ -73,6 +80,7 @@ typedef struct HPETState { uint8_t rtc_irq_level; qemu_irq pit_enabled; uint8_t num_timers; +uint32_t intcap; HPETTimer timer[HPET_MAX_TIMERS]; /* Memory-mapped, software visible registers */ @@ -663,8 +671,8 @@ static void hpet_reset(DeviceState *d) if (s-flags (1 HPET_MSI_SUPPORT)) { timer-config |= HPET_TN_FSB_CAP; } -/* advertise availability of ioapic inti2 */ -timer-config |= 0x0004ULL 32; +/* advertise availability of ioapic int */ +timer-config |= (uint64_t)s-intcap 32; timer-period = 0ULL; timer-wrap_flag = 0; } @@ -753,6 +761,7 @@ static void hpet_realize(DeviceState *dev, Error **errp) static Property hpet_device_properties[] = { DEFINE_PROP_UINT8(timers, HPETState, num_timers, HPET_MIN_TIMERS), DEFINE_PROP_BIT(msi, HPETState, flags, HPET_MSI_SUPPORT, false), +DEFINE_PROP_UINT32(intcap, HPETState, intcap, HPET_TN_INT_CAP_DEFAULT), DEFINE_PROP_END_OF_LIST(), }; -- 1.8.1.4
Re: [Qemu-devel] qemu, numa: non-contiguous cpusets
Il 29/09/2013 17:10, Borislav Petkov ha scritto: Btw, while I got your attention, on a not-really related topic: how do we feel about adding support for specifying a non-contiguous set of cpus for a numa node in qemu with the -numa option? I.e., like this, for example: x86_64-softmmu/qemu-system-x86_64 -smp 8 -numa node,nodeid=0,cpus=0\;2\;4-5 -numa node,nodeid=1,cpus=1\;3\;6-7 I think there are already patches on the list to do that, as part of the NUMA memory binding series from Wanlong Gao. Paolo The ';' needs to be escaped from the shell but I'm open for better suggestions. Here's a diff: --- diff --git a/vl.c b/vl.c index 4e709d5c1c20..82a6c8451fb0 100644 --- a/vl.c +++ b/vl.c @@ -1261,9 +1261,27 @@ char *get_boot_devices_list(size_t *size) return list; } +static int __numa_set_cpus(unsigned long *map, int start, int end) +{ +if (end = MAX_CPUMASK_BITS) { +end = MAX_CPUMASK_BITS - 1; +fprintf(stderr, +qemu: NUMA: A max of %d VCPUs are supported\n, + MAX_CPUMASK_BITS); +return -EINVAL; +} + +if (end start) { +return -EINVAL; +} + +bitmap_set(map, start, end-start+1); +return 0; +} + static void numa_node_parse_cpus(int nodenr, const char *cpus) { -char *endptr; +char *endptr, *ptr = (char *)cpus; unsigned long long value, endvalue; /* Empty CPU range strings will be considered valid, they will simply @@ -1273,7 +1291,8 @@ static void numa_node_parse_cpus(int nodenr, const char *cpus) return; } -if (parse_uint(cpus, value, endptr, 10) 0) { +fromthetop: +if (parse_uint(ptr, value, endptr, 10) 0) { goto error; } if (*endptr == '-') { @@ -1282,22 +1301,22 @@ static void numa_node_parse_cpus(int nodenr, const char *cpus) } } else if (*endptr == '\0') { endvalue = value; -} else { -goto error; -} +} else if (*endptr == ';') { + if (__numa_set_cpus(node_cpumask[nodenr], value, value) 0) { + goto error; + } + endptr++; +if (*endptr == '\0') + return; -if (endvalue = MAX_CPUMASK_BITS) { -endvalue = MAX_CPUMASK_BITS - 1; -fprintf(stderr, -qemu: NUMA: A max of %d VCPUs are supported\n, - MAX_CPUMASK_BITS); -} + ptr = endptr; -if (endvalue value) { + goto fromthetop; +} else { goto error; } -bitmap_set(node_cpumask[nodenr], value, endvalue-value+1); +__numa_set_cpus(node_cpumask[nodenr], value, endvalue); return; error: -- Thanks.
Re: [Qemu-devel] [PATCH] coroutine: add ./configure --disable-coroutine-pool
On Fri, Sep 27, 2013 at 06:49:18PM +0200, Stefan Weil wrote: Am 27.09.2013 11:11, schrieb Stefan Hajnoczi: On Fri, Sep 27, 2013 at 07:20:21AM +0200, Stefan Weil wrote: Am 11.09.2013 16:42, schrieb Stefan Hajnoczi: The 'gthread' coroutine backend was written before the freelist (aka pool) existed in qemu-coroutine.c. This means that every thread is expected to exit when its coroutine terminates. It is not possible to reuse threads from a pool. This patch automatically disables the pool when 'gthread' is used. This allows the 'gthread' backend to work again (for example, tests/test-coroutine completes successfully instead of hanging). I considered implementing thread reuse but I don't want quirks like CPU affinity differences due to coroutine threads being recycled. The 'gthread' backend is a reference backend and it's therefore okay to skip the pool optimization. Note this patch also makes it easy to toggle the pool for benchmarking purposes: ./configure --with-coroutine-backend=ucontext \ --disable-coroutine-pool Reported-by: Gabriel Kerneis gabr...@kerneis.info Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- configure| 24 qemu-coroutine.c | 34 +++--- 2 files changed, 43 insertions(+), 15 deletions(-) This patch is important for QEMU 1.5 as well, but needs some modifications there. A recent bug report for MinGW shows that the win32 coroutine needs it, too. coroutine-win32.c is designed to support reuse: static void CALLBACK coroutine_trampoline(void *co_) { Coroutine *co = co_; while (true) { co-entry(co-entry_arg); qemu_coroutine_switch(co, co-caller, COROUTINE_TERMINATE); } } We return from qemu_coroutine_switch() when the fiber is reused and simply run another iteration of the while loop. Why do you say win32 coroutines should disable the pool? Stefan QEMU MinGW binaries with coroutine pool simply crash, and disabling the pool helps for the moment until we have a better fix. That's a regression which was introduced with a patch which added the pool for win32. See this discussion thread for more details: http://lists.nongnu.org/archive/html/qemu-devel/2013-09/msg04195.html I'd like to fix the win32 coroutine backend. In your email you said it can be reproduced under wine on Linux. I wasn't able to reproduce it with this command-line: $ wine qemu-system-x86_64.exe -m 1024 -vnc :1 -L ../pc-bios -hda ../test.img Stefan: Do you have a ./configure and QEMU command-line which reproduces the assertion failure under wine? Stefan
Re: [Qemu-devel] [PATCH v5 2/5] hpet: entitle more irq pins for hpet
Il 30/09/2013 11:06, Michael S. Tsirkin ha scritto: On Mon, Sep 30, 2013 at 04:02:29PM +0800, liu ping fan wrote: On Sun, Sep 29, 2013 at 12:15 PM, Michael S. Tsirkin m...@redhat.com wrote: On Sun, Sep 29, 2013 at 11:49:41AM +0800, liu ping fan wrote: On Sun, Sep 29, 2013 at 3:56 AM, Michael S. Tsirkin m...@redhat.com wrote: On Thu, Sep 12, 2013 at 11:25:15AM +0800, Liu Ping Fan wrote: On PC, IRQ2/8 can be reserved for hpet timer 0/1. And pin 16~23 of ioapic can be dynamically assigned to hpet as guest chooses. (Will enable them after introducing pc 1.6 compat) Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com --- hw/timer/hpet.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c index 8429eb3..46903b9 100644 --- a/hw/timer/hpet.c +++ b/hw/timer/hpet.c @@ -25,6 +25,7 @@ */ #include hw/hw.h +#include hw/boards.h #include hw/i386/pc.h #include ui/console.h #include qemu/timer.h @@ -42,6 +43,12 @@ #define HPET_MSI_SUPPORT0 +/* For bug compat, using only IRQ2. Soon it will be fixed as + * 0xff0104ULL, i.e using IRQ16~23, IRQ8 and IRQ2 So users are expected to stick a bitmask of legal pins here? I think that's a bit too much rope to give to users. Don't you think? Sorry, not understand your meaning exactly. But the scene will be: guest kernel polls the ability bitmask, and pick up one pin which is not occupied or can be shared with the level-trigger and low-active. So is it rope? I merely say that it's better to make this a bool or bit property. UINT32 is too much flexibility imho. The interrupt capability is export to guest by register Tn_INT_ROUTE_CAP[63:32]. So it is useless to make them as a bit property. Do you think so? Regards Pingfan I think we merely need to support two modes: - qemu 1.6 and older compatible - compatible to actual hardware Why would we let users configure an arbitrary configuration which isn't compatible to either old qemu or real hardware? The actual setting depends on the chipset. For example, the real PIIX4 is the same as the QEMU PIIX4, only Q35 uses the new value. If in the future we had a chipset with more than 24 GSIs, you would have a third possibility. Paolo
Re: [Qemu-devel] [PATCH 0/3] hw: set irq without selecting INTx pin
On Mon, Sep 30, 2013 at 11:02:06AM +0200, Paolo Bonzini wrote: Il 30/09/2013 10:58, Michael S. Tsirkin ha scritto: As a next step, can we make pci_set_irq non-inline and make it call pci_irq_handler directly, and get rid of the irq field? What irq field? /* IRQ objects for the INTA-INTD pins. */ qemu_irq *irq; That's still used by devices that use common code for PCI and sysbus versions (e.g. USB OHCI and EHCI). Paolo Well this work wouldn't be complete without addressing them anyway. These devices would have to create their own irq in pci-specific code, along the lines of: - s-irq = dev-irq[3]; + s-irq = qemu_allocate_irqs(pci_set_irq, dev, 1); If there's more than one device like this, we should add /* Return an irq that calls pci_set_irq internally */ qemu_irq *pci_allocate_irq(PCIDevice *); -- MST
[Qemu-devel] [PATCH 1/9] roms: add 'make clean'
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- roms/Makefile | 7 +++ 1 file changed, 7 insertions(+) diff --git a/roms/Makefile b/roms/Makefile index 7a228ae..b646060 100644 --- a/roms/Makefile +++ b/roms/Makefile @@ -75,3 +75,10 @@ efi-rom-%: ipxe/src/config/local/general.h ipxe/src/config/local/%: config.ipxe.% cp $ $@ + + +clean: + rm -rf seabios/.config seabios/out + $(MAKE) $(MAKEFLAGS) -C vgabios clean + rm -f vgabios/VGABIOS-lgpl-latest* + $(MAKE) $(MAKEFLAGS) -C ipxe/src veryclean -- 1.8.3.1
[Qemu-devel] [PULL 0/9] roms: various build improvements
Hi, Here is a collection of improvements for rom building: Proper support for parallel builds, more build targets, initial support for cross builds (some targets only). please pull, Gerd The following changes since commit 53d09b761f032f50c4424e8649396a9041070bae: linux-user: Handle SOCK_CLOEXEC/NONBLOCK if unavailable on host (2013-09-24 10:47:07 +0300) are available in the git repository at: git://git.kraxel.org/qemu roms.1 for you to fetch changes up to 774e80ea1d080c608ab06a3b68d9f583644b8d85: roms: add support for building sgabios (2013-09-30 09:44:36 +0200) Gerd Hoffmann (9): roms: add 'make clean' roms: enable parallel builds for 'make lgplvgabios' roms: build lgplvgabios isavga variant roms: parallel ipxe builds roms: rewrite scripts/refresh-pxe-roms.sh roms: add rules to build slof roms: enable ipxe cross builds roms: enable parallel seabios / seavgabios builds roms: add support for building sgabios roms/Makefile | 99 ++- roms/{config.vga.cirrus = config.vga-cirrus} | 0 roms/{config.vga.isavga = config.vga-isavga} | 0 roms/{config.vga.qxl = config.vga-qxl} | 0 roms/{config.vga.stdvga = config.vga-stdvga} | 0 roms/{config.vga.vmware = config.vga-vmware} | 0 scripts/refresh-pxe-roms.sh | 80 ++ 7 files changed, 87 insertions(+), 92 deletions(-) rename roms/{config.vga.cirrus = config.vga-cirrus} (100%) rename roms/{config.vga.isavga = config.vga-isavga} (100%) rename roms/{config.vga.qxl = config.vga-qxl} (100%) rename roms/{config.vga.stdvga = config.vga-stdvga} (100%) rename roms/{config.vga.vmware = config.vga-vmware} (100%)
[Qemu-devel] [PATCH 5/9] roms: rewrite scripts/refresh-pxe-roms.sh
Just use the Makefile in roms/ Signed-off-by: Gerd Hoffmann kra...@redhat.com --- scripts/refresh-pxe-roms.sh | 80 - 1 file changed, 6 insertions(+), 74 deletions(-) diff --git a/scripts/refresh-pxe-roms.sh b/scripts/refresh-pxe-roms.sh index 14d5860..90fc0b3 100755 --- a/scripts/refresh-pxe-roms.sh +++ b/scripts/refresh-pxe-roms.sh @@ -21,79 +21,11 @@ # Usage: Run from root of qemu tree # ./scripts/refresh-pxe-roms.sh -QEMU_DIR=$PWD -ROM_DIR=pc-bios -BUILD_DIR=roms/ipxe -LOCAL_CONFIG=src/config/local/general.h - -function cleanup () -{ -if [ -n $SAVED_CONFIG ]; then -cp $SAVED_CONFIG $BUILD_DIR/$LOCAL_CONFIG -rm $SAVED_CONFIG -fi -cd $QEMU_DIR -} - -function make_rom () -{ -cd $BUILD_DIR/src - -BUILD_LOG=$(mktemp) - -echo Building $2... -make bin/$1.rom $BUILD_LOG 21 -if [ $? -ne 0 ]; then -echo Build failed -tail --lines=100 $BUILD_LOG -rm $BUILD_LOG -cleanup -exit 1 -fi -rm $BUILD_LOG - -cp bin/$1.rom $QEMU_DIR/$ROM_DIR/$2 - -cd $QEMU_DIR -} - -if [ ! -d $QEMU_DIR/$ROM_DIR ]; then -echo error: can't find $ROM_DIR directory, \ - run me from the root of the qemu tree -exit 1 -fi - -if [ ! -d $BUILD_DIR/src ]; then -echo error: $BUILD_DIR not populated, try: -echo git submodule init $BUILD_DIR -echo git submodule update $BUILD_DIR -exit 1 -fi - -if [ -e $BUILD_DIR/$LOCAL_CONFIG ]; then -SAVED_CONFIG=$(mktemp) -cp $BUILD_DIR/$LOCAL_CONFIG $SAVED_CONFIG -fi - -echo #undef BANNER_TIMEOUT $BUILD_DIR/$LOCAL_CONFIG -echo #define BANNER_TIMEOUT 0 $BUILD_DIR/$LOCAL_CONFIG - -IPXE_VERSION=$(cd $BUILD_DIR git describe --tags) -if [ -z $IPXE_VERSION ]; then -echo error: unable to retrieve git version -cleanup -exit 1 +targets=pxerom +if test -x $(which EfiRom 2/dev/null); then +targets=$targets efirom fi -echo #undef PRODUCT_NAME $BUILD_DIR/$LOCAL_CONFIG -echo #define PRODUCT_NAME \iPXE $IPXE_VERSION\ $BUILD_DIR/$LOCAL_CONFIG - -make_rom 8086100e pxe-e1000.rom -make_rom 80861209 pxe-eepro100.rom -make_rom 10500940 pxe-ne2k_pci.rom -make_rom 10222000 pxe-pcnet.rom -make_rom 10ec8139 pxe-rtl8139.rom -make_rom 1af41000 pxe-virtio.rom - -echo done -cleanup +cd roms +make -j4 $targets || exit 1 +make clean -- 1.8.3.1
[Qemu-devel] [PATCH 6/9] roms: add rules to build slof
Add some logic to detect cross compilers. Add support for make slof, which should JustWork[tm] if you are on a ppx64 machine or have a ppc64 cross compiler installed somewhere in your path. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- roms/Makefile | 22 ++ 1 file changed, 22 insertions(+) diff --git a/roms/Makefile b/roms/Makefile index 9672625..5fcc77d 100644 --- a/roms/Makefile +++ b/roms/Makefile @@ -18,6 +18,21 @@ pxe-rom-virtio efi-rom-virtio : VID := 1af4 pxe-rom-virtio efi-rom-virtio : DID := 1000 # +# cross compiler auto detection +# +path := $(subst :, ,$(PATH)) +system := $(shell uname -s | tr A-Z a-z) + +# first find cross binutils in path +find-cross-ld = $(firstword $(wildcard $(patsubst %,%/$(1)-*$(system)*-ld,$(path +# then check we have cross gcc too +find-cross-gcc = $(firstword $(wildcard $(patsubst %ld,%gcc,$(call find-cross-ld,$(1) +# finally strip off path + toolname so we get the prefix +find-cross-prefix = $(subst gcc,,$(notdir $(call find-cross-gcc,$(1 + +powerpc64_cross_prefix := $(call find-cross-prefix,powerpc64) + +# # EfiRom utility is shipped with edk2 / tianocore, in BaseTools/ # # We need that to combine multiple images (legacy bios, @@ -37,6 +52,7 @@ default: @echo pxerom -- update nic roms (bios only) @echo efirom -- update nic roms (bios+efi, this needs @echo the EfiRom utility from edk2 / tianocore) + @echo slof -- update slof.bin bios: config.seabios sh configure-seabios.sh $ @@ -90,8 +106,14 @@ ipxe/src/config/local/%: config.ipxe.% cp $ $@ +slof: + $(MAKE) $(MAKEFLAGS) -C SLOF CROSS=$(powerpc64_cross_prefix) qemu + cp SLOF/boot_rom.bin ../pc-bios/slof.bin + + clean: rm -rf seabios/.config seabios/out $(MAKE) $(MAKEFLAGS) -C vgabios clean rm -f vgabios/VGABIOS-lgpl-latest* $(MAKE) $(MAKEFLAGS) -C ipxe/src veryclean + $(MAKE) $(MAKEFLAGS) -C SLOF clean -- 1.8.3.1
[Qemu-devel] [PATCH 8/9] roms: enable parallel seabios / seavgabios builds
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- roms/Makefile | 29 ++- roms/{config.vga.cirrus = config.vga-cirrus} | 0 roms/{config.vga.isavga = config.vga-isavga} | 0 roms/{config.vga.qxl = config.vga-qxl} | 0 roms/{config.vga.stdvga = config.vga-stdvga} | 0 roms/{config.vga.vmware = config.vga-vmware} | 0 6 files changed, 19 insertions(+), 10 deletions(-) rename roms/{config.vga.cirrus = config.vga-cirrus} (100%) rename roms/{config.vga.isavga = config.vga-isavga} (100%) rename roms/{config.vga.qxl = config.vga-qxl} (100%) rename roms/{config.vga.stdvga = config.vga-stdvga} (100%) rename roms/{config.vga.vmware = config.vga-vmware} (100%) diff --git a/roms/Makefile b/roms/Makefile index 1966f04..6994873 100644 --- a/roms/Makefile +++ b/roms/Makefile @@ -55,18 +55,27 @@ default: @echo the EfiRom utility from edk2 / tianocore) @echo slof -- update slof.bin -bios: config.seabios - sh configure-seabios.sh $ - make -C seabios out/bios.bin - cp seabios/out/bios.bin ../pc-bios/bios.bin - cp seabios/out/*dsdt.aml ../pc-bios/ +bios: build-seabios-config-seabios + cp seabios/builds/seabios/bios.bin ../pc-bios/bios.bin + cp seabios/builds/seabios/*dsdt.aml ../pc-bios/ seavgabios: $(patsubst %,seavgabios-%,$(vgabios_variants)) -seavgabios-%: config.vga.% - sh configure-seabios.sh $ - make -C seabios out/vgabios.bin - cp seabios/out/vgabios.bin ../pc-bios/vgabios-$*.bin +seavgabios-isavga: build-seabios-config-vga-isavga + cp seabios/builds/vga-isavga/vgabios.bin ../pc-bios/vgabios.bin + +seavgabios-%: build-seabios-config-vga-% + cp seabios/builds/vga-$*/vgabios.bin ../pc-bios/vgabios-$*.bin + +build-seabios-config-%: config.% + mkdir -p seabios/builds/$* + cp $ seabios/builds/$*/.config + $(MAKE) $(MAKEFLAGS) -C seabios \ + KCONFIG_CONFIG=$(CURDIR)/seabios/builds/$*/.config \ + OUT=$(CURDIR)/seabios/builds/$*/ oldnoconfig + $(MAKE) $(MAKEFLAGS) -C seabios \ + KCONFIG_CONFIG=$(CURDIR)/seabios/builds/$*/.config \ + OUT=$(CURDIR)/seabios/builds/$*/ all lgplvgabios: $(patsubst %,lgplvgabios-%,$(vgabios_variants)) @@ -115,7 +124,7 @@ slof: clean: - rm -rf seabios/.config seabios/out + rm -rf seabios/.config seabios/out seabios/builds $(MAKE) $(MAKEFLAGS) -C vgabios clean rm -f vgabios/VGABIOS-lgpl-latest* $(MAKE) $(MAKEFLAGS) -C ipxe/src veryclean diff --git a/roms/config.vga.cirrus b/roms/config.vga-cirrus similarity index 100% rename from roms/config.vga.cirrus rename to roms/config.vga-cirrus diff --git a/roms/config.vga.isavga b/roms/config.vga-isavga similarity index 100% rename from roms/config.vga.isavga rename to roms/config.vga-isavga diff --git a/roms/config.vga.qxl b/roms/config.vga-qxl similarity index 100% rename from roms/config.vga.qxl rename to roms/config.vga-qxl diff --git a/roms/config.vga.stdvga b/roms/config.vga-stdvga similarity index 100% rename from roms/config.vga.stdvga rename to roms/config.vga-stdvga diff --git a/roms/config.vga.vmware b/roms/config.vga-vmware similarity index 100% rename from roms/config.vga.vmware rename to roms/config.vga-vmware -- 1.8.3.1
[Qemu-devel] [PATCH 7/9] roms: enable ipxe cross builds
--- roms/Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/roms/Makefile b/roms/Makefile index 5fcc77d..1966f04 100644 --- a/roms/Makefile +++ b/roms/Makefile @@ -31,6 +31,7 @@ find-cross-gcc = $(firstword $(wildcard $(patsubst %ld,%gcc,$(call find-cross-ld find-cross-prefix = $(subst gcc,,$(notdir $(call find-cross-gcc,$(1 powerpc64_cross_prefix := $(call find-cross-prefix,powerpc64) +x86_64_cross_prefix := $(call find-cross-prefix,x86_64) # # EfiRom utility is shipped with edk2 / tianocore, in BaseTools/ @@ -95,10 +96,12 @@ efi-rom-%: build-pxe-roms build-efi-roms build-pxe-roms: ipxe/src/config/local/general.h $(MAKE) $(MAKEFLAGS) -C ipxe/src GITVERSION= \ + CROSS_COMPILE=$(x86_64_cross_prefix) \ $(patsubst %,bin/%.rom,$(pxerom_targets)) build-efi-roms: build-pxe-roms ipxe/src/config/local/general.h $(MAKE) $(MAKEFLAGS) -C ipxe/src GITVERSION= \ + CROSS_COMPILE=$(x86_64_cross_prefix) \ $(patsubst %,bin-i386-efi/%.efidrv,$(pxerom_targets)) \ $(patsubst %,bin-x86_64-efi/%.efidrv,$(pxerom_targets)) -- 1.8.3.1
[Qemu-devel] [PATCH 2/9] roms: enable parallel builds for 'make lgplvgabios'
Recurse into vgabios once, adjust dependencies, call make using $(MAKE) $(MAKEFLAGS) so jobserver mode works. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- roms/Makefile | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/roms/Makefile b/roms/Makefile index b646060..6d4330f 100644 --- a/roms/Makefile +++ b/roms/Makefile @@ -1,5 +1,6 @@ vgabios_variants := stdvga cirrus vmware qxl +vgabios_targets := $(patsubst %,vgabios-%.bin,$(vgabios_variants)) pxerom_variants := e1000 eepro100 ne2k_pci pcnet rtl8139 virtio pxe-rom-e1000efi-rom-e1000: VID := 8086 @@ -49,12 +50,16 @@ seavgabios-%: config.vga.% make -C seabios out/vgabios.bin cp seabios/out/vgabios.bin ../pc-bios/vgabios-$*.bin + lgplvgabios: $(patsubst %,lgplvgabios-%,$(vgabios_variants)) -lgplvgabios-%: - make -C vgabios vgabios-$*.bin +lgplvgabios-%: build-lgplvgabios cp vgabios/VGABIOS-lgpl-latest.$*.bin ../pc-bios/vgabios-$*.bin +build-lgplvgabios: + $(MAKE) $(MAKEFLAGS) -C vgabios $(vgabios_targets) + + pxerom: $(patsubst %,pxe-rom-%,$(pxerom_variants)) pxe-rom-%: ipxe/src/config/local/general.h -- 1.8.3.1
Re: [Qemu-devel] [Qemu-trivial] [PATCH] block: Remove unused assignment (fixes warning from clang)
On Sun, Sep 29, 2013 at 10:15:10PM +0200, Stefan Weil wrote: Am 29.09.2013 21:44, schrieb Michael Tokarev: 28.09.2013 13:55, Stefan Weil wrote: [...] diff --git a/blockdev.c b/blockdev.c index 8aa66a9..8c83f6f 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1926,7 +1926,6 @@ void qmp_drive_mirror(const char *device, const char *target, } else { switch (mode) { case NEW_IMAGE_MODE_EXISTING: -ret = 0; break; While this one is obviously unused assignment, there's on more usage of `ret' variable in this function, -- it is to store the return value from bdrv_open(): ret = bdrv_open(target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv, local_err); if (ret 0) {... What's the rule about converting that into if() ? Thanks, /mjt Is there a rule for cases like that? This pattern is very common in QEMU code (several occurrences in blockdev.c). Should we eliminate the 'ret' variable? I don't think it's worth the effort. Me neither. There is a minor style difference between putting everything into the if statement and using a variable to split up a potentially complicated if statement. Both approaches are usually okay. Stefan
[Qemu-devel] [PATCH 4/9] roms: parallel ipxe builds
Enable parallel ipxe builds. Reduce the recursive make calls. Call recursive make properly using $(MAKE) $(MAKEFLAGS). Signed-off-by: Gerd Hoffmann kra...@redhat.com --- roms/Makefile | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/roms/Makefile b/roms/Makefile index 11d7837..9672625 100644 --- a/roms/Makefile +++ b/roms/Makefile @@ -2,6 +2,7 @@ vgabios_variants := stdvga cirrus vmware qxl isavga vgabios_targets := $(subst -isavga,,$(patsubst %,vgabios-%.bin,$(vgabios_variants))) pxerom_variants := e1000 eepro100 ne2k_pci pcnet rtl8139 virtio +pxerom_targets := 8086100e 80861209 10500940 10222000 10ec8139 1af41000 pxe-rom-e1000efi-rom-e1000: VID := 8086 pxe-rom-e1000efi-rom-e1000: DID := 100e @@ -64,22 +65,27 @@ build-lgplvgabios: pxerom: $(patsubst %,pxe-rom-%,$(pxerom_variants)) -pxe-rom-%: ipxe/src/config/local/general.h - make -C ipxe/src bin/$(VID)$(DID).rom +pxe-rom-%: build-pxe-roms cp ipxe/src/bin/$(VID)$(DID).rom ../pc-bios/pxe-$*.rom efirom: $(patsubst %,efi-rom-%,$(pxerom_variants)) -efi-rom-%: ipxe/src/config/local/general.h - make -C ipxe/src bin/$(VID)$(DID).rom - make -C ipxe/src bin-i386-efi/$(VID)$(DID).efidrv - make -C ipxe/src bin-x86_64-efi/$(VID)$(DID).efidrv +efi-rom-%: build-pxe-roms build-efi-roms $(EFIROM) -f 0x$(VID) -i 0x$(DID) -l 0x02 \ -b ipxe/src/bin/$(VID)$(DID).rom \ -ec ipxe/src/bin-i386-efi/$(VID)$(DID).efidrv \ -ec ipxe/src/bin-x86_64-efi/$(VID)$(DID).efidrv \ -o ../pc-bios/efi-$*.rom +build-pxe-roms: ipxe/src/config/local/general.h + $(MAKE) $(MAKEFLAGS) -C ipxe/src GITVERSION= \ + $(patsubst %,bin/%.rom,$(pxerom_targets)) + +build-efi-roms: build-pxe-roms ipxe/src/config/local/general.h + $(MAKE) $(MAKEFLAGS) -C ipxe/src GITVERSION= \ + $(patsubst %,bin-i386-efi/%.efidrv,$(pxerom_targets)) \ + $(patsubst %,bin-x86_64-efi/%.efidrv,$(pxerom_targets)) + ipxe/src/config/local/%: config.ipxe.% cp $ $@ -- 1.8.3.1
[Qemu-devel] [PATCH 9/9] roms: add support for building sgabios
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- roms/Makefile | 9 + 1 file changed, 9 insertions(+) diff --git a/roms/Makefile b/roms/Makefile index 6994873..10d5a65 100644 --- a/roms/Makefile +++ b/roms/Makefile @@ -50,6 +50,7 @@ default: @echo bios -- update bios.bin (seabios) @echo seavgabios -- update vgabios binaries (seabios) @echo lgplvgabios-- update vgabios binaries (lgpl) + @echo sgabios-- update sgabios binaries @echo pxerom -- update nic roms (bios only) @echo efirom -- update nic roms (bios+efi, this needs @echo the EfiRom utility from edk2 / tianocore) @@ -89,6 +90,12 @@ build-lgplvgabios: $(MAKE) $(MAKEFLAGS) -C vgabios $(vgabios_targets) +.PHONY: sgabios +sgabios: + $(MAKE) $(MAKEFLAGS) -C sgabios + cp sgabios/sgabios.bin ../pc-bios + + pxerom: $(patsubst %,pxe-rom-%,$(pxerom_variants)) pxe-rom-%: build-pxe-roms @@ -127,5 +134,7 @@ clean: rm -rf seabios/.config seabios/out seabios/builds $(MAKE) $(MAKEFLAGS) -C vgabios clean rm -f vgabios/VGABIOS-lgpl-latest* + $(MAKE) $(MAKEFLAGS) -C sgabios clean + rm -f sgabios/.depend $(MAKE) $(MAKEFLAGS) -C ipxe/src veryclean $(MAKE) $(MAKEFLAGS) -C SLOF clean -- 1.8.3.1
[Qemu-devel] [PATCH 3/9] roms: build lgplvgabios isavga variant
Add logic to also build+install the isavga vgabios variant. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- roms/Makefile | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/roms/Makefile b/roms/Makefile index 6d4330f..11d7837 100644 --- a/roms/Makefile +++ b/roms/Makefile @@ -1,6 +1,6 @@ -vgabios_variants := stdvga cirrus vmware qxl -vgabios_targets := $(patsubst %,vgabios-%.bin,$(vgabios_variants)) +vgabios_variants := stdvga cirrus vmware qxl isavga +vgabios_targets := $(subst -isavga,,$(patsubst %,vgabios-%.bin,$(vgabios_variants))) pxerom_variants := e1000 eepro100 ne2k_pci pcnet rtl8139 virtio pxe-rom-e1000efi-rom-e1000: VID := 8086 @@ -53,6 +53,8 @@ seavgabios-%: config.vga.% lgplvgabios: $(patsubst %,lgplvgabios-%,$(vgabios_variants)) +lgplvgabios-isavga: build-lgplvgabios + cp vgabios/VGABIOS-lgpl-latest.bin ../pc-bios/vgabios.bin lgplvgabios-%: build-lgplvgabios cp vgabios/VGABIOS-lgpl-latest.$*.bin ../pc-bios/vgabios-$*.bin -- 1.8.3.1
Re: [Qemu-devel] [PATCH] block: use correct filename for error report
ping? On Tue, Sep 24, 2013 at 8:12 PM, Max Reitz mre...@redhat.com wrote: On 2013-09-24 12:14, Dunrong Huang wrote: The content filename point to will be erased by qemu_opts_absorb_qdict() in raw_open_common() in drv-bdrv_file_open() So it's better to use bs-filename. Signed-off-by: Dunrong Huang riegama...@gmail.com --- block.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Max Reitz mre...@redhat.com -- Best Regards, Dunrong Huang Homepage: http://mathslinux.org
[Qemu-devel] [PULL 0/1] seabios update to 1.7.3.2
Hi, New seabios release is out, here comes the update for qemu. please pull, Gerd The following changes since commit 53d09b761f032f50c4424e8649396a9041070bae: linux-user: Handle SOCK_CLOEXEC/NONBLOCK if unavailable on host (2013-09-24 10:47:07 +0300) are available in the git repository at: git://git.kraxel.org/qemu seabios-1.7.3.2 for you to fetch changes up to 1cf9412b3b583b59a1ac131609cbf673662ee7eb: update seabios from 1.7.2.2 to 1.7.3.2 (2013-09-30 11:18:02 +0200) Gerd Hoffmann (1): update seabios from 1.7.2.2 to 1.7.3.2 pc-bios/acpi-dsdt.aml | Bin 4407 - 4407 bytes pc-bios/bios.bin | Bin 131072 - 131072 bytes pc-bios/q35-acpi-dsdt.aml | Bin 7344 - 7344 bytes roms/seabios | 2 +- 4 files changed, 1 insertion(+), 1 deletion(-)
Re: [Qemu-devel] [PATCH v5 2/5] hpet: entitle more irq pins for hpet
On Mon, Sep 30, 2013 at 11:06:48AM +0200, Paolo Bonzini wrote: Il 30/09/2013 11:06, Michael S. Tsirkin ha scritto: On Mon, Sep 30, 2013 at 04:02:29PM +0800, liu ping fan wrote: On Sun, Sep 29, 2013 at 12:15 PM, Michael S. Tsirkin m...@redhat.com wrote: On Sun, Sep 29, 2013 at 11:49:41AM +0800, liu ping fan wrote: On Sun, Sep 29, 2013 at 3:56 AM, Michael S. Tsirkin m...@redhat.com wrote: On Thu, Sep 12, 2013 at 11:25:15AM +0800, Liu Ping Fan wrote: On PC, IRQ2/8 can be reserved for hpet timer 0/1. And pin 16~23 of ioapic can be dynamically assigned to hpet as guest chooses. (Will enable them after introducing pc 1.6 compat) Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com --- hw/timer/hpet.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c index 8429eb3..46903b9 100644 --- a/hw/timer/hpet.c +++ b/hw/timer/hpet.c @@ -25,6 +25,7 @@ */ #include hw/hw.h +#include hw/boards.h #include hw/i386/pc.h #include ui/console.h #include qemu/timer.h @@ -42,6 +43,12 @@ #define HPET_MSI_SUPPORT0 +/* For bug compat, using only IRQ2. Soon it will be fixed as + * 0xff0104ULL, i.e using IRQ16~23, IRQ8 and IRQ2 So users are expected to stick a bitmask of legal pins here? I think that's a bit too much rope to give to users. Don't you think? Sorry, not understand your meaning exactly. But the scene will be: guest kernel polls the ability bitmask, and pick up one pin which is not occupied or can be shared with the level-trigger and low-active. So is it rope? I merely say that it's better to make this a bool or bit property. UINT32 is too much flexibility imho. The interrupt capability is export to guest by register Tn_INT_ROUTE_CAP[63:32]. So it is useless to make them as a bit property. Do you think so? Regards Pingfan I think we merely need to support two modes: - qemu 1.6 and older compatible - compatible to actual hardware Why would we let users configure an arbitrary configuration which isn't compatible to either old qemu or real hardware? The actual setting depends on the chipset. For example, the real PIIX4 is the same as the QEMU PIIX4, only Q35 uses the new value. If in the future we had a chipset with more than 24 GSIs, you would have a third possibility. Paolo I was really only talking about q35 here. I thought it's ugly that users can control intcap directly. Can object_set_property be used after qdev_try_create? PIIX has another issue: the default value in hpet is really Q35 specific, that's also kind of ugly, isn't it? -- MST
Re: [Qemu-devel] [PATCH 0/3] hw: set irq without selecting INTx pin
On Mon, 2013-09-30 at 12:14 +0300, Michael S. Tsirkin wrote: On Mon, Sep 30, 2013 at 11:02:06AM +0200, Paolo Bonzini wrote: Il 30/09/2013 10:58, Michael S. Tsirkin ha scritto: As a next step, can we make pci_set_irq non-inline and make it call pci_irq_handler directly, and get rid of the irq field? What irq field? /* IRQ objects for the INTA-INTD pins. */ qemu_irq *irq; That's still used by devices that use common code for PCI and sysbus versions (e.g. USB OHCI and EHCI). Paolo Well this work wouldn't be complete without addressing them anyway. These devices would have to create their own irq in pci-specific code, along the lines of: This irq field is used also in places where pci_set_irq(PCIDevice dev, level) can't infer the INTx: - PCIExpress: qemu_set_irq(dev-irq[dev-exp.hpev_intx],dev-exp.hpev_notified); - vmxnet3 device: qemu_set_irq(d-irq[int_idx], 1); What approach should be used here? Thanks, Marcel - s-irq = dev-irq[3]; + s-irq = qemu_allocate_irqs(pci_set_irq, dev, 1); If there's more than one device like this, we should add /* Return an irq that calls pci_set_irq internally */ qemu_irq *pci_allocate_irq(PCIDevice *);
Re: [Qemu-devel] [PATCH 2/3] qcow2: Free allocated L2 cluster on error
On 2013-09-27 16:54, Kevin Wolf wrote: Am 25.09.2013 um 16:37 hat Max Reitz geschrieben: If an error occurs in l2_allocate, the allocated (but unused) L2 cluster should be freed. Signed-off-by: Max Reitz mre...@redhat.com --- block/qcow2-cluster.c | 4 1 file changed, 4 insertions(+) This needs an update of the reference output for test case 026 (both for -nocache and writethrough). Yes, right. Most of the changes look expected and good, like cluster leaks disappearing. With -nocache, however, there are a few cases that failed previously and result in successful writes now. It would be interesting to see the explanation for these before we merge the patch. I personally don't see this cases. Could you give an example? Max
Re: [Qemu-devel] qemu, numa: non-contiguous cpusets
On Mon, Sep 30, 2013 at 11:05:10AM +0200, Paolo Bonzini wrote: I think there are already patches on the list to do that, as part of the NUMA memory binding series from Wanlong Gao. Yeah: https://lists.gnu.org/archive/html/qemu-devel/2013-09/msg02833.html Although I don't see from it how the syntax for -cpus will look like from that QAPI magic except that it is an + '*cpus': ['uint16'], :-) Thanks. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. --
Re: [Qemu-devel] Attaching PCI devices to the PCIe root complex
Michael S. Tsirkin m...@redhat.com writes: On Fri, Sep 27, 2013 at 07:06:44PM +0200, Markus Armbruster wrote: Marcel Apfelbaum marcel.apfelb...@gmail.com writes: On Wed, 2013-09-25 at 10:01 +0300, Michael S. Tsirkin wrote: On Tue, Sep 24, 2013 at 06:01:02AM -0400, Laine Stump wrote: When I added support for the Q35-based machinetypes to libvirt, I specifically prohibited attaching any PCI devices (with the exception of graphics controllers) to the PCIe root complex, That's wrong I think. Anything attached to RC is an integrated endpoint, and these can be PCI devices. I couldn't find on PCIe spec any mention that Root Complex Integrated EndPoint must be PCIe. But, from spec 1.3.2.3: - A Root Complex Integrated Endpoint must not require I/O resources claimed through BAR(s). - A Root Complex Integrated Endpoint must not generate I/O Requests. - A Root Complex Integrated Endpoint is required to support MSI or MSI-X or both if an interrupt resource is requested. I suppose that this restriction can be removed for PCI devices that 1. Actually work when plugged in into RC Integrated EndPoint 2. Respond to the above limitations and had planned to prevent attaching them to PCIe root ports (ioh3420 device) and PCIe downstream switch ports (xio-3130 device) as well. I did this because, even though qemu currently allows attaching a normal PCI device in any of these three places, the restriction exists for real hardware and I didn't see any guarantee that qemu wouldn't add the restriction in the future in order to more closely emulate real hardware. However, since I did that, I've learned that many of the qemu pci devices really should be considered as pci or pcie. Gerd Hoffman lists some of these cases in a bug he filed against libvirt: https://bugzilla.redhat.com/show_bug.cgi?id=1003983 I would like to loosen up the restrictions in libvirt, but want to make sure that I don't allow something that could later be forbidden by qemu (thus creating a compatibility problem during upgrades). Beyond Gerd's specific requests to allow ehci, uhci, and hda controllers to attach to PCIe ports, are there any other devices that I specifically should or shouldn't allow? (I would rather be conservative in what I allow - it's easy to allow more things later, but nearly impossible to revoke permission once it's been allowed). For the moment I would not remove any restrictions, but only the ones requested and verified by somebody. IMO, we really need to grow an interface to query this kind of thing. Basically libvirt needs to know: 1. for (libvirt) controllers: what kind of devices can be plugged in 2. for devices (controller is also a device) - to which controllers can it be plugged in - does it support hot-plug? 3. implicit controllers of the machine types (q35 - pcie-root, i440fx - pci-root) All the above must be exported to libvirt Implementation options: 1. Add a compliance field on PCI/PCIe devices and controllers stating if it supports PCI/PCIe or both (and maybe hot-plug) - consider plug type + compliance to figure out whether a plug can go into a socket 2. Use Markus Armbruster idea of introducing a concept of plug and sockets: - dividing the devices into adapters and plugs - adding sockets to bridges(buses?). In this way it would be clear which devices can connect to bridges This isn't actually my idea. It's how things are designed to work in qdev, at least in my admittedly limited understanding of qdev. In traditional qdev, a device has exactly one plug (its bus type, shown by -device help), and it may have one ore more buses. Each bus has a type, and you can plug a device only into a bus of the matching type. This was too limiting, and is not how things work now. As far as I know, libvirt already understands that a device can only plug into a matching bus. In my understanding of where we're headed with qdev, things are / will become more general, yet stay really simple: * A device can have an arbitrary number of sockets and plugs. * Each socket / plug has a type. * A plug can go into a socket of the same type, and only there. Pretty straightforward generalization of traditional qdev. I wouldn't expect libvirt to have serious trouble coping with it, as long as we provide the necessary information on device models' plugs and sockets. In this framework, there's no such thing as a device model that can plug either into a PCI or a PCIe socket. Makes sense to me, because there's no such thing in the physical world, either. Instead, and just like in the physical world, you have one separate device variant per desired plug type. Two types of bus is not how things work in real world though. In real world there are 3 types of express bus besides classical pci bus, and limitations
Re: [Qemu-devel] [Qemu-trivial] [PATCH] migration: Fix compiler warning ('caps' may be used uninitialized)
Stefan Weil s...@weilnetz.de writes: Am 29.09.2013 22:13, schrieb Michael Tokarev: 29.09.2013 19:41, Stefan Weil wrote: The QEMU buildbot default_i386_debian_6_0 shows this warning: CCmigration.o migration.c: In function 'qmp_query_migrate_capabilities': migration.c:149: warning: 'caps' may be used uninitialized in this function Gah, how disgusting. The code is correct, yet gcc complains needlessly... That's not the first time where we help the compiler by modifying the code. It's also not the first time where attempting to help the compiler made code less readable, or even less correct. So let's be just as careful as with real changes.
Re: [Qemu-devel] [PATCH 0/3] hw: set irq without selecting INTx pin
On Mon, Sep 30, 2013 at 12:43:20PM +0300, Marcel Apfelbaum wrote: On Mon, 2013-09-30 at 12:14 +0300, Michael S. Tsirkin wrote: On Mon, Sep 30, 2013 at 11:02:06AM +0200, Paolo Bonzini wrote: Il 30/09/2013 10:58, Michael S. Tsirkin ha scritto: As a next step, can we make pci_set_irq non-inline and make it call pci_irq_handler directly, and get rid of the irq field? What irq field? /* IRQ objects for the INTA-INTD pins. */ qemu_irq *irq; That's still used by devices that use common code for PCI and sysbus versions (e.g. USB OHCI and EHCI). Paolo Well this work wouldn't be complete without addressing them anyway. These devices would have to create their own irq in pci-specific code, along the lines of: This irq field is used also in places where pci_set_irq(PCIDevice dev, level) can't infer the INTx: - PCIExpress: qemu_set_irq(dev-irq[dev-exp.hpev_intx],dev-exp.hpev_notified); Well the spec says, explicitly: 6.7.3.4. Software Notification of Hot-Plug Events ... Note that all other interrupt sources within the same Function will assert the same virtual INTx wire when requesting service. I read this to mean that this is a bug, and it should simply use pci_set_irq like all other devices. - vmxnet3 device: qemu_set_irq(d-irq[int_idx], 1); What approach should be used here? Thanks, Marcel - s-irq = dev-irq[3]; + s-irq = qemu_allocate_irqs(pci_set_irq, dev, 1); If there's more than one device like this, we should add /* Return an irq that calls pci_set_irq internally */ qemu_irq *pci_allocate_irq(PCIDevice *);
[Qemu-devel] [PATCH uq/master] kvmvapic: Prevent reading beyond the end of guest RAM
rom_state_paddr is guest provided (caller address of outw(VAPIC_PORT) + writen 16-bit value) and can be influenced to point beyond the end of the host memory backing the guest's RAM. Make sure we do not use this pointer to actually read beyond the limits. Reading arbitrary guest bytes is harmless, the guest kernel has to manage access to this I/O port anyway. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- hw/i386/kvmvapic.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c index 1c2dbf5..2d87600 100644 --- a/hw/i386/kvmvapic.c +++ b/hw/i386/kvmvapic.c @@ -596,6 +596,9 @@ static int vapic_map_rom_writable(VAPICROMState *s) section = memory_region_find(as, 0, 1); /* read ROM size from RAM region */ +if (rom_paddr + 2 = memory_region_size(section.mr)) { +return -1; +} ram = memory_region_get_ram_ptr(section.mr); rom_size = ram[rom_paddr + 2] * ROM_BLOCK_SIZE; if (rom_size == 0) { -- 1.8.1.1.298.ge7eed54
Re: [Qemu-devel] [PATCH 0/3] hw: set irq without selecting INTx pin
On Mon, 2013-09-30 at 13:10 +0300, Michael S. Tsirkin wrote: On Mon, Sep 30, 2013 at 12:43:20PM +0300, Marcel Apfelbaum wrote: On Mon, 2013-09-30 at 12:14 +0300, Michael S. Tsirkin wrote: On Mon, Sep 30, 2013 at 11:02:06AM +0200, Paolo Bonzini wrote: Il 30/09/2013 10:58, Michael S. Tsirkin ha scritto: As a next step, can we make pci_set_irq non-inline and make it call pci_irq_handler directly, and get rid of the irq field? What irq field? /* IRQ objects for the INTA-INTD pins. */ qemu_irq *irq; That's still used by devices that use common code for PCI and sysbus versions (e.g. USB OHCI and EHCI). Paolo Well this work wouldn't be complete without addressing them anyway. These devices would have to create their own irq in pci-specific code, along the lines of: This irq field is used also in places where pci_set_irq(PCIDevice dev, level) can't infer the INTx: - PCIExpress: qemu_set_irq(dev-irq[dev-exp.hpev_intx],dev-exp.hpev_notified); Well the spec says, explicitly: 6.7.3.4. Software Notification of Hot-Plug Events ... Note that all other interrupt sources within the same Function will assert the same virtual INTx wire when requesting service. I read this to mean that this is a bug, and it should simply use pci_set_irq like all other devices. Thanks! That means that hpev_intx is not necessary at all. By the way, aer_intx used by Advanced Error Reporting is also unnecessary. (6.2.4.1.2 has the same note) I will remove the above fields from PCIExpressDevice. Marcel - vmxnet3 device: qemu_set_irq(d-irq[int_idx], 1); What approach should be used here? Thanks, Marcel - s-irq = dev-irq[3]; + s-irq = qemu_allocate_irqs(pci_set_irq, dev, 1); If there's more than one device like this, we should add /* Return an irq that calls pci_set_irq internally */ qemu_irq *pci_allocate_irq(PCIDevice *);
Re: [Qemu-devel] Attaching PCI devices to the PCIe root complex
On 09/30/2013 05:55 AM, Markus Armbruster wrote: Michael S. Tsirkin m...@redhat.com writes: I never heard of a pci/pci express one but it's not impossible I think. PCI on one side of the card, PCIe on the other, and a switchable backplate? Weird :) Again, I can't see why we'd want to model this, even if it existed. Unfortunately that's what's been done, so libvirt will have to deal with it, even after qemu gets it fixed. The current interface munges together all PCIish connectors, and the result is a mess: users can't see which device can be plugged into which socket. Libvirt needs to know, and it has grown a bunch of hardcoded ad hoc rules, which aren't quite right. The good news is that libvirt has only recently started dealing with anything other than vanilla PCI slots. The bad news is that it has. I guess everything should work out okay if we just keep the current wild guess code around, but only fall back on it if the new more accurate information is unavailable.
Re: [Qemu-devel] Attaching PCI devices to the PCIe root complex
On Mon, Sep 30, 2013 at 11:55:47AM +0200, Markus Armbruster wrote: Michael S. Tsirkin m...@redhat.com writes: On Fri, Sep 27, 2013 at 07:06:44PM +0200, Markus Armbruster wrote: Marcel Apfelbaum marcel.apfelb...@gmail.com writes: On Wed, 2013-09-25 at 10:01 +0300, Michael S. Tsirkin wrote: On Tue, Sep 24, 2013 at 06:01:02AM -0400, Laine Stump wrote: When I added support for the Q35-based machinetypes to libvirt, I specifically prohibited attaching any PCI devices (with the exception of graphics controllers) to the PCIe root complex, That's wrong I think. Anything attached to RC is an integrated endpoint, and these can be PCI devices. I couldn't find on PCIe spec any mention that Root Complex Integrated EndPoint must be PCIe. But, from spec 1.3.2.3: - A Root Complex Integrated Endpoint must not require I/O resources claimed through BAR(s). - A Root Complex Integrated Endpoint must not generate I/O Requests. - A Root Complex Integrated Endpoint is required to support MSI or MSI-X or both if an interrupt resource is requested. I suppose that this restriction can be removed for PCI devices that 1. Actually work when plugged in into RC Integrated EndPoint 2. Respond to the above limitations and had planned to prevent attaching them to PCIe root ports (ioh3420 device) and PCIe downstream switch ports (xio-3130 device) as well. I did this because, even though qemu currently allows attaching a normal PCI device in any of these three places, the restriction exists for real hardware and I didn't see any guarantee that qemu wouldn't add the restriction in the future in order to more closely emulate real hardware. However, since I did that, I've learned that many of the qemu pci devices really should be considered as pci or pcie. Gerd Hoffman lists some of these cases in a bug he filed against libvirt: https://bugzilla.redhat.com/show_bug.cgi?id=1003983 I would like to loosen up the restrictions in libvirt, but want to make sure that I don't allow something that could later be forbidden by qemu (thus creating a compatibility problem during upgrades). Beyond Gerd's specific requests to allow ehci, uhci, and hda controllers to attach to PCIe ports, are there any other devices that I specifically should or shouldn't allow? (I would rather be conservative in what I allow - it's easy to allow more things later, but nearly impossible to revoke permission once it's been allowed). For the moment I would not remove any restrictions, but only the ones requested and verified by somebody. IMO, we really need to grow an interface to query this kind of thing. Basically libvirt needs to know: 1. for (libvirt) controllers: what kind of devices can be plugged in 2. for devices (controller is also a device) - to which controllers can it be plugged in - does it support hot-plug? 3. implicit controllers of the machine types (q35 - pcie-root, i440fx - pci-root) All the above must be exported to libvirt Implementation options: 1. Add a compliance field on PCI/PCIe devices and controllers stating if it supports PCI/PCIe or both (and maybe hot-plug) - consider plug type + compliance to figure out whether a plug can go into a socket 2. Use Markus Armbruster idea of introducing a concept of plug and sockets: - dividing the devices into adapters and plugs - adding sockets to bridges(buses?). In this way it would be clear which devices can connect to bridges This isn't actually my idea. It's how things are designed to work in qdev, at least in my admittedly limited understanding of qdev. In traditional qdev, a device has exactly one plug (its bus type, shown by -device help), and it may have one ore more buses. Each bus has a type, and you can plug a device only into a bus of the matching type. This was too limiting, and is not how things work now. As far as I know, libvirt already understands that a device can only plug into a matching bus. In my understanding of where we're headed with qdev, things are / will become more general, yet stay really simple: * A device can have an arbitrary number of sockets and plugs. * Each socket / plug has a type. * A plug can go into a socket of the same type, and only there. Pretty straightforward generalization of traditional qdev. I wouldn't expect libvirt to have serious trouble coping with it, as long as we provide the necessary information on device models' plugs and sockets. In this framework, there's no such thing as a device model that can plug either into a PCI or a PCIe socket. Makes sense to me, because there's no such thing in the physical world, either. Instead, and just like in the physical world, you have one
[Qemu-devel] [RfC PATCH 0/2] seabios: going to 256k size
Hi, With the seabios update to the next master branch relase (NOT the stable branch pull sent out earlier today) seabios rom size will cross the 128k boundary and we have to deal with that. So here is a RfC patch series with one aproach to handle this: We'll go build two seabios binaries, one 128k (will less features), one 256k (full featured). Machine types 1.7+ will use the 256k bios while older machine types will continue to use the 128k sized one. Series applies on top of the roms: various build improvements pull request sent out earlier today. Comments? cheers, Gerd Gerd Hoffmann (2): roms: build two seabios binaries machine type dependant bios hw/i386/pc_piix.c| 1 + hw/i386/pc_q35.c | 1 + hw/i386/pc_sysfw.c | 4 ++-- include/hw/i386/pc.h | 2 ++ roms/Makefile| 7 --- roms/config.seabios | 1 - roms/config.seabios-128k | 5 + roms/config.seabios-256k | 3 +++ 8 files changed, 18 insertions(+), 6 deletions(-) delete mode 100644 roms/config.seabios create mode 100644 roms/config.seabios-128k create mode 100644 roms/config.seabios-256k -- 1.8.3.1
[Qemu-devel] [PATCH 1/2] roms: build two seabios binaries
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- roms/Makefile| 7 --- roms/config.seabios | 1 - roms/config.seabios-128k | 5 + roms/config.seabios-256k | 3 +++ 4 files changed, 12 insertions(+), 4 deletions(-) delete mode 100644 roms/config.seabios create mode 100644 roms/config.seabios-128k create mode 100644 roms/config.seabios-256k diff --git a/roms/Makefile b/roms/Makefile index 10d5a65..1d84230 100644 --- a/roms/Makefile +++ b/roms/Makefile @@ -56,9 +56,10 @@ default: @echo the EfiRom utility from edk2 / tianocore) @echo slof -- update slof.bin -bios: build-seabios-config-seabios - cp seabios/builds/seabios/bios.bin ../pc-bios/bios.bin - cp seabios/builds/seabios/*dsdt.aml ../pc-bios/ +bios: build-seabios-config-seabios-128k build-seabios-config-seabios-256k + cp seabios/builds/seabios-128k/bios.bin ../pc-bios/bios-128k.bin + cp seabios/builds/seabios-256k/bios.bin ../pc-bios/bios-256k.bin + cp seabios/builds/seabios-256k/src/fw/*dsdt.aml ../pc-bios/ seavgabios: $(patsubst %,seavgabios-%,$(vgabios_variants)) diff --git a/roms/config.seabios b/roms/config.seabios deleted file mode 100644 index c373b87..000 --- a/roms/config.seabios +++ /dev/null @@ -1 +0,0 @@ -# empty, default config works for us diff --git a/roms/config.seabios-128k b/roms/config.seabios-128k new file mode 100644 index 000..23ca812 --- /dev/null +++ b/roms/config.seabios-128k @@ -0,0 +1,5 @@ +# for qemu machine types 1.6 + older +# need to turn off features (xhci) to make it fit into 128k +CONFIG_QEMU=y +CONFIG_ROM_SIZE=128 +CONFIG_USB_XHCI=n diff --git a/roms/config.seabios-256k b/roms/config.seabios-256k new file mode 100644 index 000..cc37a78 --- /dev/null +++ b/roms/config.seabios-256k @@ -0,0 +1,3 @@ +# for qemu machine types 1.7 + newer +CONFIG_QEMU=y +CONFIG_ROM_SIZE=256 -- 1.8.3.1
[Qemu-devel] [PATCH 2/2] machine type dependant bios
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/i386/pc_piix.c| 1 + hw/i386/pc_q35.c | 1 + hw/i386/pc_sysfw.c | 4 ++-- include/hw/i386/pc.h | 2 ++ 4 files changed, 6 insertions(+), 2 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 907792b..35188a5 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -238,6 +238,7 @@ static void pc_init_pci(QEMUMachineInitArgs *args) static void pc_compat_1_6(QEMUMachineInitArgs *args) { +pc_default_bios_name = bios-128k.bin; has_pci_info = false; rom_file_in_ram = false; } diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index ca84e1c..e8b0168 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -222,6 +222,7 @@ static void pc_q35_init(QEMUMachineInitArgs *args) static void pc_compat_1_6(QEMUMachineInitArgs *args) { +pc_default_bios_name = bios-128k.bin; has_pci_info = false; rom_file_in_ram = false; } diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c index e917c83..deb8869 100644 --- a/hw/i386/pc_sysfw.c +++ b/hw/i386/pc_sysfw.c @@ -34,7 +34,7 @@ #include hw/block/flash.h #include sysemu/kvm.h -#define BIOS_FILENAME bios.bin +const char *pc_default_bios_name = bios-256k.bin; typedef struct PcSysFwDevice { SysBusDevice busdev; @@ -112,7 +112,7 @@ static void old_pc_system_rom_init(MemoryRegion *rom_memory, bool isapc_ram_fw) /* BIOS load */ if (bios_name == NULL) { -bios_name = BIOS_FILENAME; +bios_name = pc_default_bios_name; } filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name); if (filename) { diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 9b2ddc4..cf9e47e 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -12,6 +12,8 @@ /* PC-style peripherals (also used by other machines). */ +extern const char *pc_default_bios_name; + typedef struct PcPciInfo { Range w32; Range w64; -- 1.8.3.1
Re: [Qemu-devel] [PATCH uq/master] kvmvapic: Prevent reading beyond the end of guest RAM
On Mon, Sep 30, 2013 at 12:35:13PM +0200, Jan Kiszka wrote: rom_state_paddr is guest provided (caller address of outw(VAPIC_PORT) + writen 16-bit value) and can be influenced to point beyond the end of the host memory backing the guest's RAM. Make sure we do not use this pointer to actually read beyond the limits. Reading arbitrary guest bytes is harmless, the guest kernel has to manage access to this I/O port anyway. Signed-off-by: Jan Kiszka jan.kis...@siemens.com Acked-by: Michael S. Tsirkin m...@redhat.com --- hw/i386/kvmvapic.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c index 1c2dbf5..2d87600 100644 --- a/hw/i386/kvmvapic.c +++ b/hw/i386/kvmvapic.c @@ -596,6 +596,9 @@ static int vapic_map_rom_writable(VAPICROMState *s) section = memory_region_find(as, 0, 1); /* read ROM size from RAM region */ +if (rom_paddr + 2 = memory_region_size(section.mr)) { +return -1; +} ram = memory_region_get_ram_ptr(section.mr); rom_size = ram[rom_paddr + 2] * ROM_BLOCK_SIZE; if (rom_size == 0) { -- 1.8.1.1.298.ge7eed54
[Qemu-devel] mail to Paul Brook p...@codesourcery.com bouncing [postmas...@relay1.mentorg.com: Delivery Status Notification (Failure)]
Anyone knows what's going on? If no mail should be sent to p...@codesourcery.com, let's add .mailcap so get_maintainer doesn't suggest this address. - Forwarded message from postmas...@relay1.mentorg.com - Date: Mon, 30 Sep 2013 03:08:22 -0700 From: postmas...@relay1.mentorg.com To: m...@redhat.com Subject: Delivery Status Notification (Failure) Message-ID: eql24hcd30004d...@svr-orw-fem-01.mgc.mentorg.com This is an automatically generated Delivery Status Notification. Delivery to the following recipients failed. paul_br...@mentor.com Reporting-MTA: dns;svr-orw-fem-01.mgc.mentorg.com Received-From-MTA: dns;relay1.mentorg.com Arrival-Date: Mon, 30 Sep 2013 03:08:21 -0700 Final-Recipient: rfc822;paul_br...@mentor.com Action: failed Status: 5.7.1 X-Display-Name: Brook, Paul Date: Mon, 30 Sep 2013 13:10:35 +0300 From: Michael S. Tsirkin m...@redhat.com To: Marcel Apfelbaum marce...@redhat.com Cc: Paolo Bonzini pbonz...@redhat.com, qemu-devel@nongnu.org qemu-devel@nongnu.org, kra...@redhat.com, p...@codesourcery.com, anth...@codemonkey.ws, afaer...@suse.de, s...@weilnetz.de, peter.crosthwa...@xilinx.com, stefa...@redhat.com, jasow...@redhat.com, dk...@verizon.com, alex.william...@redhat.com Subject: Re: [Qemu-devel] [PATCH 0/3] hw: set irq without selecting INTx pin Message-ID: 20130930101035.gb20...@redhat.com In-Reply-To: 1380534200.3439.8.camel@localhost.localdomain On Mon, Sep 30, 2013 at 12:43:20PM +0300, Marcel Apfelbaum wrote: On Mon, 2013-09-30 at 12:14 +0300, Michael S. Tsirkin wrote: On Mon, Sep 30, 2013 at 11:02:06AM +0200, Paolo Bonzini wrote: Il 30/09/2013 10:58, Michael S. Tsirkin ha scritto: As a next step, can we make pci_set_irq non-inline and make it call pci_irq_handler directly, and get rid of the irq field? What irq field? /* IRQ objects for the INTA-INTD pins. */ qemu_irq *irq; That's still used by devices that use common code for PCI and sysbus versions (e.g. USB OHCI and EHCI). Paolo Well this work wouldn't be complete without addressing them anyway. These devices would have to create their own irq in pci-specific code, along the lines of: This irq field is used also in places where pci_set_irq(PCIDevice dev, level) can't infer the INTx: - PCIExpress: qemu_set_irq(dev-irq[dev-exp.hpev_intx],dev-exp.hpev_notified); Well the spec says, explicitly: 6.7.3.4. Software Notification of Hot-Plug Events ... Note that all other interrupt sources within the same Function will assert the same virtual INTx wire when requesting service. I read this to mean that this is a bug, and it should simply use pci_set_irq like all other devices. - vmxnet3 device: qemu_set_irq(d-irq[int_idx], 1); What approach should be used here? Thanks, Marcel - s-irq = dev-irq[3]; + s-irq = qemu_allocate_irqs(pci_set_irq, dev, 1); If there's more than one device like this, we should add /* Return an irq that calls pci_set_irq internally */ qemu_irq *pci_allocate_irq(PCIDevice *); - End forwarded message -
Re: [Qemu-devel] [PATCH 0/3] hw: set irq without selecting INTx pin
Il 30/09/2013 12:39, Marcel Apfelbaum ha scritto: Thanks! That means that hpev_intx is not necessary at all. By the way, aer_intx used by Advanced Error Reporting is also unnecessary. (6.2.4.1.2 has the same note) I will remove the above fields from PCIExpressDevice. vmxnet is also buggy and should just use pci_set_irq for INTx. Paolo
Re: [Qemu-devel] [PATCH 1/2] roms: build two seabios binaries
Il 30/09/2013 12:46, Gerd Hoffmann ha scritto: Signed-off-by: Gerd Hoffmann kra...@redhat.com --- roms/Makefile| 7 --- roms/config.seabios | 1 - roms/config.seabios-128k | 5 + roms/config.seabios-256k | 3 +++ 4 files changed, 12 insertions(+), 4 deletions(-) delete mode 100644 roms/config.seabios create mode 100644 roms/config.seabios-128k create mode 100644 roms/config.seabios-256k diff --git a/roms/Makefile b/roms/Makefile index 10d5a65..1d84230 100644 --- a/roms/Makefile +++ b/roms/Makefile @@ -56,9 +56,10 @@ default: @echo the EfiRom utility from edk2 / tianocore) @echo slof -- update slof.bin -bios: build-seabios-config-seabios - cp seabios/builds/seabios/bios.bin ../pc-bios/bios.bin - cp seabios/builds/seabios/*dsdt.aml ../pc-bios/ +bios: build-seabios-config-seabios-128k build-seabios-config-seabios-256k + cp seabios/builds/seabios-128k/bios.bin ../pc-bios/bios-128k.bin + cp seabios/builds/seabios-256k/bios.bin ../pc-bios/bios-256k.bin + cp seabios/builds/seabios-256k/src/fw/*dsdt.aml ../pc-bios/ seavgabios: $(patsubst %,seavgabios-%,$(vgabios_variants)) diff --git a/roms/config.seabios b/roms/config.seabios deleted file mode 100644 index c373b87..000 --- a/roms/config.seabios +++ /dev/null @@ -1 +0,0 @@ -# empty, default config works for us diff --git a/roms/config.seabios-128k b/roms/config.seabios-128k new file mode 100644 index 000..23ca812 --- /dev/null +++ b/roms/config.seabios-128k @@ -0,0 +1,5 @@ +# for qemu machine types 1.6 + older +# need to turn off features (xhci) to make it fit into 128k +CONFIG_QEMU=y +CONFIG_ROM_SIZE=128 +CONFIG_USB_XHCI=n Is it enough to disable Xen or perhaps OHCI? diff --git a/roms/config.seabios-256k b/roms/config.seabios-256k new file mode 100644 index 000..cc37a78 --- /dev/null +++ b/roms/config.seabios-256k @@ -0,0 +1,3 @@ +# for qemu machine types 1.7 + newer +CONFIG_QEMU=y +CONFIG_ROM_SIZE=256 Is there already an option to remove ACPI tables generation? Should it be enabled for 1.7, irrespective of the 128k-256k switch? Paolo
Re: [Qemu-devel] [PATCH v3] spapr-rtas: fix h_rtas parameters reading
On 27.09.2013, at 10:10, Alexey Kardashevskiy wrote: On the real hardware, RTAS is called in real mode and therefore top 4 bits of the address passed in the call are ignored. So does the patch. This converts h_rtas() to use existing rtas_ld() handlers. This fixed rtas_ld()/rtas_st() to ignore top 4 bits. Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru Thanks, applied to ppc-next. Alex
Re: [Qemu-devel] [PATCH v2] spapr: Add ibm, purr property on power7 and newer
On 27.09.2013, at 10:11, Alexey Kardashevskiy wrote: PAPR+ says that no ibm,purr tells the guest that H_PURR is not supported. However some guests still try calling H_PURR on POWER7 unless the property is present and equal to 0. This adds the property for CPUs supporting the PURR special register. Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru Thanks, applied to ppc-next. Alex
Re: [Qemu-devel] [PATCH 2/3] qcow2: Free allocated L2 cluster on error
Am 30.09.2013 um 11:48 hat Max Reitz geschrieben: On 2013-09-27 16:54, Kevin Wolf wrote: Am 25.09.2013 um 16:37 hat Max Reitz geschrieben: If an error occurs in l2_allocate, the allocated (but unused) L2 cluster should be freed. Signed-off-by: Max Reitz mre...@redhat.com --- block/qcow2-cluster.c | 4 1 file changed, 4 insertions(+) This needs an update of the reference output for test case 026 (both for -nocache and writethrough). Yes, right. Most of the changes look expected and good, like cluster leaks disappearing. With -nocache, however, there are a few cases that failed previously and result in successful writes now. It would be interesting to see the explanation for these before we merge the patch. I personally don't see this cases. Could you give an example? Weird, I can't reproduce it any more. I guess it was a problem on my side then. Kevin
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
On 27.09.2013, at 10:06, Alexey Kardashevskiy wrote: To be able to boot on newer hardware that the software support, PowerISA defines a logical PVR, one per every PowerISA specification version from 2.04. This adds the compat option which takes values 205 or 206 and forces QEMU to boot the guest with a logical PVR (CPU_POWERPC_LOGICAL_2_05 or CPU_POWERPC_LOGICAL_2_06). The guest reads the logical PVR value from cpu-version property of a CPU device node. Cc: Nikunj A Dadhania nik...@linux.vnet.ibm.com Cc: Andreas Färber afaer...@suse.de Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru --- hw/ppc/spapr.c | 40 include/hw/ppc/spapr.h | 2 ++ target-ppc/cpu-models.h | 10 ++ target-ppc/cpu.h| 3 +++ target-ppc/kvm.c| 2 ++ vl.c| 4 6 files changed, 61 insertions(+) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index a09a1d9..737452d 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -33,6 +33,7 @@ #include sysemu/kvm.h #include kvm_ppc.h #include mmu-hash64.h +#include cpu-models.h #include hw/boards.h #include hw/ppc/ppc.h @@ -196,6 +197,26 @@ static XICSState *xics_system_init(int nr_servers, int nr_irqs) return icp; } +static void spapr_compat_mode_init(sPAPREnvironment *spapr) +{ +QemuOpts *machine_opts = qemu_get_machine_opts(); +uint64_t compat = qemu_opt_get_number(machine_opts, compat, 0); + +switch (compat) { +case 0: +break; +case 205: +spapr-arch_compat = CPU_POWERPC_LOGICAL_2_05; +break; +case 206: +spapr-arch_compat = CPU_POWERPC_LOGICAL_2_06; Does it make sense to declare compat mode a number or would a string be easier for users? I can imagine that -machine compat=power6 is easier to understand for a user than -machine compat=205. +break; +default: +perror(Unsupported mode, only are 205, 206 supported\n); +break; +} +} + static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr) { int ret = 0, offset; @@ -206,6 +227,7 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr) CPU_FOREACH(cpu) { DeviceClass *dc = DEVICE_GET_CLASS(cpu); +CPUPPCState *env = (POWERPC_CPU(cpu)-env); uint32_t associativity[] = {cpu_to_be32(0x5), cpu_to_be32(0x0), cpu_to_be32(0x0), @@ -238,6 +260,14 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr) if (ret 0) { return ret; } + +if (env-arch_compat) { +ret = fdt_setprop(fdt, offset, cpu-version, + env-arch_compat, sizeof(env-arch_compat)); +if (ret 0) { +return ret; +} +} } return ret; } @@ -1145,6 +1175,8 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args) spapr = g_malloc0(sizeof(*spapr)); QLIST_INIT(spapr-phbs); +spapr_compat_mode_init(spapr); + cpu_ppc_hypercall = emulate_spapr_hypercall; /* Allocate RMA if necessary */ @@ -1226,6 +1258,14 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args) xics_cpu_setup(spapr-icp, cpu); +/* + * If compat mode is set in the command line, pass it to CPU so KVM + * will be able to set it in the host kernel. + */ +if (spapr-arch_compat) { +env-arch_compat = spapr-arch_compat; You should set the compat mode in KVM here, rather than doing it in the put_registers call which gets invoked on every register sync. Or can the guest change the mode? Also, we need to handle failure. If the kernel can not set the CPU to 2.05 mode for example (IIRC POWER8 doesn't allow you to) we should bail out here. And then there's the TCG question. We either have to disable CPU features similar to how we handle it in KVM (by setting and honoring the respective bits in PCR) or we need to bail out too and declare compat mode unsupported for TCG. And then there's the fact that the kernel interface isn't upstream in a way that +} + qemu_register_reset(spapr_cpu_reset, cpu); } diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index ca175b0..201c578 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -34,6 +34,8 @@ typedef struct sPAPREnvironment { uint32_t epow_irq; Notifier epow_notifier; +uint32_t arch_compat;/* Compatible PVR from the command line */ + /* Migration state */ int htab_save_index; bool htab_first_pass; diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h index 49ba4a4..d7c033c 100644 --- a/target-ppc/cpu-models.h +++ b/target-ppc/cpu-models.h @@ -583,6 +583,16 @@ enum { CPU_POWERPC_RS64II = 0x0034, CPU_POWERPC_RS64III
Re: [Qemu-devel] [PATCH v7] powerpc: add PVR mask support
On 27.09.2013, at 10:05, Alexey Kardashevskiy wrote: IBM POWERPC processors encode PVR as a CPU family in higher 16 bits and a CPU version in lower 16 bits. Since there is no significant change in behavior between versions, there is no point to add every single CPU version in QEMU's CPU list. Also, new CPU versions of already supported CPU won't break the existing code. This adds PVR value/mask support for KVM, i.e. for -cpu host option. As CPU family class name for POWER7 is POWER7-family, there is no need to touch aliases. Cc: Andreas Färber afaer...@suse.de Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru Looks reasonable to me, but I'll wait for an ack from Andreas. Alex
[Qemu-devel] [PATCH V3] block: Add BlockDriver.bdrv_check_ext_snapshot.
This field is used by blkverify to disable external snapshots creation. I will also be used by block filters like quorum to disable external snapshots creation. Signed-off-by: Benoit Canet ben...@irqsave.net --- block.c | 14 ++ block/blkverify.c | 2 ++ blockdev.c| 5 + include/block/block.h | 14 ++ include/block/block_int.h | 6 ++ 5 files changed, 41 insertions(+) diff --git a/block.c b/block.c index 93e113a..d54b5e2 100644 --- a/block.c +++ b/block.c @@ -4632,3 +4632,17 @@ int bdrv_amend_options(BlockDriverState *bs, QEMUOptionParameter *options) } return bs-drv-bdrv_amend_options(bs, options); } + +ExtSnapshotPerm bdrv_check_ext_snapshot(BlockDriverState *bs) +{ +/* external snashots are allowed by defaults */ +if (!bs-drv-bdrv_check_ext_snapshot) { +return EXT_SNAPSHOT_ALLOWED; +} +return bs-drv-bdrv_check_ext_snapshot(bs); +} + +ExtSnapshotPerm bdrv_check_ext_snapshot_forbidden(BlockDriverState *bs) +{ +return EXT_SNAPSHOT_FORBIDDEN; +} diff --git a/block/blkverify.c b/block/blkverify.c index 2077d8a..b64c638 100644 --- a/block/blkverify.c +++ b/block/blkverify.c @@ -313,6 +313,8 @@ static BlockDriver bdrv_blkverify = { .bdrv_aio_readv = blkverify_aio_readv, .bdrv_aio_writev= blkverify_aio_writev, .bdrv_aio_flush = blkverify_aio_flush, + +.bdrv_check_ext_snapshot = bdrv_check_ext_snapshot_forbidden, }; static void bdrv_blkverify_init(void) diff --git a/blockdev.c b/blockdev.c index 8aa66a9..92029d8 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1131,6 +1131,11 @@ static void external_snapshot_prepare(BlkTransactionState *common, } } +if (bdrv_check_ext_snapshot(state-old_bs) != EXT_SNAPSHOT_ALLOWED) { +error_set(errp, QERR_FEATURE_DISABLED, snapshot); +return; +} + flags = state-old_bs-open_flags; /* create new image w/backing file */ diff --git a/include/block/block.h b/include/block/block.h index f808550..2bf0e12 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -244,6 +244,20 @@ int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix); int bdrv_amend_options(BlockDriverState *bs_new, QEMUOptionParameter *options); +/* external snapshots */ + +typedef enum { +EXT_SNAPSHOT_ALLOWED, +EXT_SNAPSHOT_FORBIDDEN, +} ExtSnapshotPerm; + +/* return EXT_SNAPSHOT_ALLOWED if external snapshot is allowed + * return EXT_SNAPSHOT_FORBIDEN if external snapshot is forbiden + */ +ExtSnapshotPerm bdrv_check_ext_snapshot(BlockDriverState *bs); +/* helper used to forbid external snapshots like in blkverify */ +ExtSnapshotPerm bdrv_check_ext_snapshot_forbidden(BlockDriverState *bs); + /* async block I/O */ typedef void BlockDriverDirtyHandler(BlockDriverState *bs, int64_t sector, int sector_num); diff --git a/include/block/block_int.h b/include/block/block_int.h index 211087a..e6a14ae 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -67,6 +67,12 @@ typedef struct BdrvTrackedRequest { struct BlockDriver { const char *format_name; int instance_size; + +/* if not defined external snapshots are allowed + * future block filters will query their children to build the response + */ +ExtSnapshotPerm (*bdrv_check_ext_snapshot)(BlockDriverState *bs); + int (*bdrv_probe)(const uint8_t *buf, int buf_size, const char *filename); int (*bdrv_probe_device)(const char *filename); -- 1.8.1.2
[Qemu-devel] [PATCH V3] disable blkverify external snapshot creation
Hello, Here is the new version of the snapshot forbidding patch. v3: functions return an enum [Jeff] rename forbiding function [Kevin] v2: Use NULL fields to avoid having to fill the new field in every BlockDriver [Jeff] Rename the field [Kevin] Benoît Canet (1): block: Add BlockDriver.bdrv_check_ext_snapshot. block.c | 14 ++ block/blkverify.c | 2 ++ blockdev.c| 5 + include/block/block.h | 14 ++ include/block/block_int.h | 6 ++ 5 files changed, 41 insertions(+) -- 1.8.1.2
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
Il 30/09/2013 13:25, Alexander Graf ha scritto: On 27.09.2013, at 10:06, Alexey Kardashevskiy wrote: To be able to boot on newer hardware that the software support, PowerISA defines a logical PVR, one per every PowerISA specification version from 2.04. This adds the compat option which takes values 205 or 206 and forces QEMU to boot the guest with a logical PVR (CPU_POWERPC_LOGICAL_2_05 or CPU_POWERPC_LOGICAL_2_06). The guest reads the logical PVR value from cpu-version property of a CPU device node. Why is the option under -machine instead of -cpu? Paolo
[Qemu-devel] [PATCH v4 1/7] block: add bdrv_common_ancestor()
This function finds the common ancestor in backing chain of two BDSes. If there's no common ancestor, NULL is returned. Signed-off-by: Fam Zheng f...@redhat.com --- block.c | 15 +++ include/block/block.h | 2 ++ 2 files changed, 17 insertions(+) diff --git a/block.c b/block.c index ea4956d..db1381d 100644 --- a/block.c +++ b/block.c @@ -3036,6 +3036,21 @@ BlockDriverState *bdrv_find(const char *name) return NULL; } +BlockDriverState *bdrv_common_ancestor(BlockDriverState *bs1, + BlockDriverState *bs2) +{ +/* TODO: higher efficiency with reverse linked backing chain */ +BlockDriverState *base1, *base2; +for (base1 = bs1; base1; base1 = base1-backing_hd) { +for (base2 = bs2; base2; base2 = base2-backing_hd) { +if (base1 == base2) { +return base1; +} +} +} +return NULL; +} + BlockDriverState *bdrv_next(BlockDriverState *bs) { if (!bs) { diff --git a/include/block/block.h b/include/block/block.h index f808550..e095f16 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -321,6 +321,8 @@ void bdrv_lock_medium(BlockDriverState *bs, bool locked); void bdrv_eject(BlockDriverState *bs, bool eject_flag); const char *bdrv_get_format_name(BlockDriverState *bs); BlockDriverState *bdrv_find(const char *name); +BlockDriverState *bdrv_common_ancestor(BlockDriverState *bs1, + BlockDriverState *bs2); BlockDriverState *bdrv_next(BlockDriverState *bs); void bdrv_iterate(void (*it)(void *opaque, BlockDriverState *bs), void *opaque); -- 1.8.3.1
[Qemu-devel] [PATCH v4 0/7] block: allow commit active as top
Previously live commit of active block device is not supported, this series implements it and updates corresponding qemu-iotests cases. v4: Rewrite to reuse block/mirror.c. When committing the active layer, the job is internally a mirror job with type name faked to commit. When the job completes, the BDSes are swapped, so the base image become active and [top, base) dropped. Fam Zheng (7): block: add bdrv_common_ancestor() qmp: add internal sync mode common to mirror_start mirror: don't close target mirror: Add commit_job_type to perform commit with mirror code commit: support commit active layer commit: remove unused check qemu-iotests: update test cases for commit active block.c| 15 ++ block/commit.c | 7 - block/mirror.c | 29 +++--- blockdev.c | 49 +-- include/block/block.h | 2 ++ include/block/block_int.h | 2 ++ qapi-schema.json | 2 +- tests/qemu-iotests/040 | 73 -- tests/qemu-iotests/041 | 5 tests/qemu-iotests/041.out | 4 +-- 10 files changed, 129 insertions(+), 59 deletions(-) -- 1.8.3.1
[Qemu-devel] [PATCH v4 5/7] commit: support commit active layer
If active is top, it will be mirrored to base, (with block/mirror.c code), then the image is switched when user completes the block job. Signed-off-by: Fam Zheng f...@redhat.com --- blockdev.c | 42 -- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/blockdev.c b/blockdev.c index 032913e..f730e6f 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1663,6 +1663,30 @@ void qmp_block_stream(const char *device, bool has_base, trace_qmp_block_stream(bs, bs-job); } +typedef struct { +BlockDriverState *bs; +BlockDriverState *base; +} commit_active_info_t; + +static void commit_active_cb(void *_info, int ret) +{ +commit_active_info_t *info = _info; +BlockDriverState *p; +assert(info-bs); +assert(info-base); +assert(info-base-backing_hd); +if (!ret) { +/* Mirror code already swapped bs and base, we drop the bs loop chain + * formed by the swap: break the loop chain, trigger the chain unref. + */ +p = info-base-backing_hd; +info-base-backing_hd = NULL; +bdrv_unref(p); +} +block_job_cb(info-bs, ret); +g_free(info); +} + void qmp_block_commit(const char *device, bool has_base, const char *base, const char *top, bool has_speed, int64_t speed, @@ -1710,8 +1734,22 @@ void qmp_block_commit(const char *device, return; } -commit_start(bs, base_bs, top_bs, speed, on_error, block_job_cb, bs, -local_err); +if (top_bs == bs) { +commit_active_info_t *info = g_malloc(sizeof(commit_active_info_t)); +info-bs = bs; +info-base = base_bs; +if (bdrv_reopen(base_bs, bs-open_flags, local_err)) { +error_propagate(errp, local_err); +return; +} +bdrv_ref(base_bs); +mirror_start(bs, base_bs, speed, 0, 0, MIRROR_SYNC_MODE_COMMON, + on_error, on_error, true, + commit_active_cb, info, local_err); +} else { +commit_start(bs, base_bs, top_bs, speed, on_error, block_job_cb, bs, +local_err); +} if (local_err != NULL) { error_propagate(errp, local_err); return; -- 1.8.3.1
[Qemu-devel] [PATCH v4 2/7] qmp: add internal sync mode common to mirror_start
This adds a new sync mode common which only copies data that is above the common ancestor of source and target. In general, this could be useful in cases like: base_bs --- common_ancestor --- foo --- bar ---source \ \--- target Where data in foo, bar and source will be copied to target, once such common backing_hd sharing is introduced. For now, we could use a special case: If target is the ancestor of source, like, base_bs --- target --- foo --- bar ---source The data in foo, bar and source will be copied to target, like drive-commit, and when they are synced, the source bs replaces target bs. This is specifically useful for block commit of active layer. This mode is not available (-ENOTSUP) from QMP interface, it is only used internally by block commit code. Signed-off-by: Fam Zheng f...@redhat.com --- block/mirror.c | 16 ++-- blockdev.c | 5 + qapi-schema.json | 2 +- tests/qemu-iotests/041 | 5 + tests/qemu-iotests/041.out | 4 ++-- 5 files changed, 27 insertions(+), 5 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index 6e7a274..fdc7fa8 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -31,6 +31,7 @@ typedef struct MirrorBlockJob { BlockJob common; RateLimit limit; BlockDriverState *target; +BlockDriverState *base; MirrorSyncMode mode; BlockdevOnError on_source_error, on_target_error; bool synced; @@ -334,8 +335,7 @@ static void coroutine_fn mirror_run(void *opaque) if (s-mode != MIRROR_SYNC_MODE_NONE) { /* First part, loop on the sectors and initialize the dirty bitmap. */ -BlockDriverState *base; -base = s-mode == MIRROR_SYNC_MODE_FULL ? NULL : bs-backing_hd; +BlockDriverState *base = s-base; for (sector_num = 0; sector_num end; ) { int64_t next = (sector_num | (sectors_per_chunk - 1)) + 1; ret = bdrv_is_allocated_above(bs, base, @@ -541,6 +541,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target, void *opaque, Error **errp) { MirrorBlockJob *s; +BlockDriverState *base = NULL; if (granularity == 0) { /* Choose the default granularity based on the target file's cluster @@ -563,6 +564,16 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target, return; } +if (mode == MIRROR_SYNC_MODE_COMMON) { +base = bdrv_common_ancestor(bs, target); +if (!base) { +error_setg(errp, source and target has no common ancestor); +return; +} +} else if (mode == MIRROR_SYNC_MODE_TOP) { +base = bs-backing_hd; +} + s = block_job_create(mirror_job_type, bs, speed, cb, opaque, errp); if (!s) { return; @@ -572,6 +583,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target, s-on_target_error = on_target_error; s-target = target; s-mode = mode; +s-base = base; s-granularity = granularity; s-buf_size = MAX(buf_size, granularity); diff --git a/blockdev.c b/blockdev.c index 8aa66a9..3acd330 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1906,6 +1906,11 @@ void qmp_drive_mirror(const char *device, const char *target, return; } +if (sync == MIRROR_SYNC_MODE_COMMON) { +error_set(errp, QERR_UNSUPPORTED); +return; +} + flags = bs-open_flags | BDRV_O_RDWR; source = bs-backing_hd; if (!source sync == MIRROR_SYNC_MODE_TOP) { diff --git a/qapi-schema.json b/qapi-schema.json index 145eca8..6addf49 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1363,7 +1363,7 @@ # Since: 1.3 ## { 'enum': 'MirrorSyncMode', - 'data': ['top', 'full', 'none'] } + 'data': ['top', 'full', 'none', 'common'] } ## # @BlockJobInfo: diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 index 6661c03..86b3251 100755 --- a/tests/qemu-iotests/041 +++ b/tests/qemu-iotests/041 @@ -206,6 +206,11 @@ class TestSingleDrive(ImageMirroringTestCase): target=target_img) self.assert_qmp(result, 'error/class', 'DeviceNotFound') +def test_common(self): +result = self.vm.qmp('drive-mirror', device='drive0', sync='common', + target=target_img) +self.assert_qmp(result, 'error/class', 'GenericError') + class TestMirrorNoBacking(ImageMirroringTestCase): image_len = 2 * 1024 * 1024 # MB diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out index 42314e9..4fd1c2d 100644 --- a/tests/qemu-iotests/041.out +++ b/tests/qemu-iotests/041.out @@ -1,5 +1,5 @@ - +. -- -Ran 24 tests +Ran 25 tests OK -- 1.8.3.1
[Qemu-devel] [PATCH v4 6/7] commit: remove unused check
We support top == active for commit now, remove the check which is dead code now. Signed-off-by: Fam Zheng f...@redhat.com --- block/commit.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/block/commit.c b/block/commit.c index ac4b7cc..086f8c9 100644 --- a/block/commit.c +++ b/block/commit.c @@ -198,13 +198,6 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base, return; } -/* Once we support top == active layer, remove this check */ -if (top == bs) { -error_setg(errp, - Top image as the active layer is currently unsupported); -return; -} - if (top == base) { error_setg(errp, Invalid files for merge: top and base are the same); return; -- 1.8.3.1
[Qemu-devel] [PATCH v4 7/7] qemu-iotests: update test cases for commit active
Factor out commit test common logic into super class, and update test of committing the active image. Signed-off-by: Fam Zheng f...@redhat.com --- tests/qemu-iotests/040 | 73 +- 1 file changed, 31 insertions(+), 42 deletions(-) diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 index aad535a..902df0d 100755 --- a/tests/qemu-iotests/040 +++ b/tests/qemu-iotests/040 @@ -63,6 +63,28 @@ class ImageCommitTestCase(iotests.QMPTestCase): i = i + 512 file.close() +def run_commit_test(self, top, base, is_active=False): +self.assert_no_active_commit() +result = self.vm.qmp('block-commit', device='drive0', top=top, base=base) +self.assert_qmp(result, 'return', {}) + +completed = False +while not completed: +for event in self.vm.get_qmp_events(wait=True): +if event['event'] == 'BLOCK_JOB_COMPLETED': +self.assert_qmp(event, 'data/type', 'commit') +self.assert_qmp(event, 'data/device', 'drive0') +self.assert_qmp(event, 'data/offset', self.image_len) +self.assert_qmp(event, 'data/len', self.image_len) +completed = True +elif event['event'] == 'BLOCK_JOB_READY': +self.assert_qmp(event, 'data/type', 'commit') +self.assert_qmp(event, 'data/device', 'drive0') +self.assert_qmp(event, 'data/len', self.image_len) +self.vm.qmp('block-job-complete', device='drive0') + +self.assert_no_active_commit() +self.vm.shutdown() class TestSingleDrive(ImageCommitTestCase): image_len = 1 * 1024 * 1024 @@ -84,23 +106,7 @@ class TestSingleDrive(ImageCommitTestCase): os.remove(backing_img) def test_commit(self): -self.assert_no_active_commit() -result = self.vm.qmp('block-commit', device='drive0', top='%s' % mid_img) -self.assert_qmp(result, 'return', {}) - -completed = False -while not completed: -for event in self.vm.get_qmp_events(wait=True): -if event['event'] == 'BLOCK_JOB_COMPLETED': -self.assert_qmp(event, 'data/type', 'commit') -self.assert_qmp(event, 'data/device', 'drive0') -self.assert_qmp(event, 'data/offset', self.image_len) -self.assert_qmp(event, 'data/len', self.image_len) -completed = True - -self.assert_no_active_commit() -self.vm.shutdown() - +self.run_commit_test(mid_img, backing_img) self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', backing_img).find(verification failed)) self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', backing_img).find(verification failed)) @@ -127,10 +133,9 @@ class TestSingleDrive(ImageCommitTestCase): self.assert_qmp(result, 'error/desc', 'Base \'badfile\' not found') def test_top_is_active(self): -self.assert_no_active_commit() -result = self.vm.qmp('block-commit', device='drive0', top='%s' % test_img, base='%s' % backing_img) -self.assert_qmp(result, 'error/class', 'GenericError') -self.assert_qmp(result, 'error/desc', 'Top image as the active layer is currently unsupported') +self.run_commit_test(test_img, backing_img, True) +self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', backing_img).find(verification failed)) +self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', backing_img).find(verification failed)) def test_top_and_base_reversed(self): self.assert_no_active_commit() @@ -191,23 +196,7 @@ class TestRelativePaths(ImageCommitTestCase): raise def test_commit(self): -self.assert_no_active_commit() -result = self.vm.qmp('block-commit', device='drive0', top='%s' % self.mid_img) -self.assert_qmp(result, 'return', {}) - -completed = False -while not completed: -for event in self.vm.get_qmp_events(wait=True): -if event['event'] == 'BLOCK_JOB_COMPLETED': -self.assert_qmp(event, 'data/type', 'commit') -self.assert_qmp(event, 'data/device', 'drive0') -self.assert_qmp(event, 'data/offset', self.image_len) -self.assert_qmp(event, 'data/len', self.image_len) -completed = True - -self.assert_no_active_commit() -self.vm.shutdown() - +self.run_commit_test(self.mid_img, self.backing_img) self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', self.backing_img_abs).find(verification failed)) self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', self.backing_img_abs).find(verification failed)) @@ -234,10 +223,9 @@ class
[Qemu-devel] [PATCH v4 3/7] mirror: don't close target
Let reference count manage target and don't call bdrv_close here. Signed-off-by: Fam Zheng f...@redhat.com --- block/mirror.c | 1 - 1 file changed, 1 deletion(-) diff --git a/block/mirror.c b/block/mirror.c index fdc7fa8..af6851f 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -479,7 +479,6 @@ immediate_exit: } bdrv_swap(s-target, s-common.bs); } -bdrv_close(s-target); bdrv_unref(s-target); block_job_completed(s-common, ret); } -- 1.8.3.1
[Qemu-devel] [PATCH v4 4/7] mirror: Add commit_job_type to perform commit with mirror code
Commit active layer will be implemented in block/mirror.c, prepare a new job type to let it have a right type name for the user. Signed-off-by: Fam Zheng f...@redhat.com --- block/mirror.c| 12 +++- blockdev.c| 2 +- include/block/block_int.h | 2 ++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index af6851f..20dcfb6 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -532,10 +532,19 @@ static const BlockJobType mirror_job_type = { .complete = mirror_complete, }; +static const BlockJobType commit_job_type = { +.instance_size = sizeof(MirrorBlockJob), +.job_type = commit, +.set_speed = mirror_set_speed, +.iostatus_reset= mirror_iostatus_reset, +.complete = mirror_complete, +}; + void mirror_start(BlockDriverState *bs, BlockDriverState *target, int64_t speed, int64_t granularity, int64_t buf_size, MirrorSyncMode mode, BlockdevOnError on_source_error, BlockdevOnError on_target_error, + bool commit_job, BlockDriverCompletionFunc *cb, void *opaque, Error **errp) { @@ -573,7 +582,8 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target, base = bs-backing_hd; } -s = block_job_create(mirror_job_type, bs, speed, cb, opaque, errp); +s = block_job_create(commit_job ? commit_job_type : mirror_job_type, + bs, speed, cb, opaque, errp); if (!s) { return; } diff --git a/blockdev.c b/blockdev.c index 3acd330..032913e 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1963,7 +1963,7 @@ void qmp_drive_mirror(const char *device, const char *target, } mirror_start(bs, target_bs, speed, granularity, buf_size, sync, - on_source_error, on_target_error, + on_source_error, on_target_error, false, block_job_cb, bs, local_err); if (local_err != NULL) { bdrv_unref(target_bs); diff --git a/include/block/block_int.h b/include/block/block_int.h index 3eeb6fe..77a6295 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -377,6 +377,7 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base, * @mode: Whether to collapse all images in the chain to the target. * @on_source_error: The action to take upon error reading from the source. * @on_target_error: The action to take upon error writing to the target. + * @commit_job: Whether the job type string should be commit. * @cb: Completion function for the job. * @opaque: Opaque pointer value passed to @cb. * @errp: Error object. @@ -390,6 +391,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target, int64_t speed, int64_t granularity, int64_t buf_size, MirrorSyncMode mode, BlockdevOnError on_source_error, BlockdevOnError on_target_error, + bool commit_job, BlockDriverCompletionFunc *cb, void *opaque, Error **errp); -- 1.8.3.1
Re: [Qemu-devel] [PATCH v4 5/7] commit: support commit active layer
Il 30/09/2013 14:02, Fam Zheng ha scritto: +/* Mirror code already swapped bs and base, we drop the bs loop chain + * formed by the swap: break the loop chain, trigger the chain unref. + */ +p = info-base-backing_hd; +info-base-backing_hd = NULL; +bdrv_unref(p); Should this code be in mirror_run? Perhaps extracting it into a new function together with the preceding lines: if (bdrv_get_flags(s-target) != bdrv_get_flags(s-common.bs)) { bdrv_reopen(s-target, bdrv_get_flags(s-common.bs), NULL); } bdrv_swap(s-target, s-common.bs); Same for this: +if (bdrv_reopen(base_bs, bs-open_flags, local_err)) { +error_propagate(errp, local_err); +return; +} +bdrv_ref(base_bs); Perhaps you can have an internal function mirror_start_job and two front-ends mirror_start + commit_active_start that wrap it. Similarly, mirror_start_job can take the base BDS and a bool instead of MirrorSyncMode (removing the need for MIRROR_SYNC_MODE_COMMON). But apart from these details, the series looks very nice. Paolo
Re: [Qemu-devel] [PATCH v4 6/7] commit: remove unused check
Il 30/09/2013 14:02, Fam Zheng ha scritto: We support top == active for commit now, remove the check which is dead code now. Signed-off-by: Fam Zheng f...@redhat.com --- block/commit.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/block/commit.c b/block/commit.c index ac4b7cc..086f8c9 100644 --- a/block/commit.c +++ b/block/commit.c @@ -198,13 +198,6 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base, return; } -/* Once we support top == active layer, remove this check */ -if (top == bs) { -error_setg(errp, - Top image as the active layer is currently unsupported); -return; -} - if (top == base) { error_setg(errp, Invalid files for merge: top and base are the same); return; Perhaps this could even become an assertion, or it could take care of calling commit_active_start? Paolo
Re: [Qemu-devel] qemu, numa: non-contiguous cpusets
Il 30/09/2013 11:49, Borislav Petkov ha scritto: Yeah: https://lists.gnu.org/archive/html/qemu-devel/2013-09/msg02833.html Although I don't see from it how the syntax for -cpus will look like from that QAPI magic except that it is an + '*cpus': ['uint16'], It's -numa node,cpus=0-1,cpus=4-5. Paolo
Re: [Qemu-devel] [PATCH v4 2/3] qemu-timer: make qemu_timer_mod_ns() and qemu_timer_del() thread-safe
Stefan Hajnoczi stefa...@redhat.com writes: Introduce QEMUTimerList-active_timers_lock to protect the linked list of active timers. This allows qemu_timer_mod_ns() to be called from any thread. Note that vm_clock is not thread-safe and its use of qemu_clock_has_timers() works fine today but is also not thread-safe. The purpose of this patch is to eventually let device models set or cancel timers from a vcpu thread without holding the global mutex. I've applied this set to Paolo's rcu tree - I see a couple of routines that appear to need the active_timers_lock: (line 137 of qemu-timer.c in my tree) void qemu_clock_notify(QEMUClockType type) { QEMUTimerList *timer_list; QEMUClock *clock = qemu_clock_ptr(type); QLIST_FOREACH(timer_list, clock-timerlists, list) { timerlist_notify(timer_list); } } (line 228 of qemu-timer.c in my tree) int64_t qemu_clock_deadline_ns_all(QEMUClockType type) { int64_t deadline = -1; QEMUTimerList *timer_list; QEMUClock *clock = qemu_clock_ptr(type); QLIST_FOREACH(timer_list, clock-timerlists, list) { deadline = qemu_soonest_timeout(deadline, timerlist_deadline_ns(timer_list)); } return deadline; } I think these functions are always called now with the BQL held, so I wonder if they are good candidates for RCU? Mike Signed-off-by: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- include/qemu/timer.h | 17 ++ qemu-timer.c | 87 2 files changed, 85 insertions(+), 19 deletions(-) diff --git a/include/qemu/timer.h b/include/qemu/timer.h index e4934dd..b58903b 100644 --- a/include/qemu/timer.h +++ b/include/qemu/timer.h @@ -115,6 +115,10 @@ static inline int64_t qemu_clock_get_us(QEMUClockType type) * Determines whether a clock's default timer list * has timers attached * + * Note that this function should not be used when other threads also access + * the timer list. The return value may be outdated by the time it is acted + * upon. + * * Returns: true if the clock's default timer list * has timers attached */ @@ -271,6 +275,10 @@ void timerlist_free(QEMUTimerList *timer_list); * * Determine whether a timer list has active timers * + * Note that this function should not be used when other threads also access + * the timer list. The return value may be outdated by the time it is acted + * upon. + * * Returns: true if the timer list has timers. */ bool timerlist_has_timers(QEMUTimerList *timer_list); @@ -512,6 +520,9 @@ void timer_free(QEMUTimer *ts); * @ts: the timer * * Delete a timer from the active list. + * + * This function is thread-safe but the timer and its timer list must not be + * freed while this function is running. */ void timer_del(QEMUTimer *ts); @@ -521,6 +532,9 @@ void timer_del(QEMUTimer *ts); * @expire_time: the expiry time in nanoseconds * * Modify a timer to expire at @expire_time + * + * This function is thread-safe but the timer and its timer list must not be + * freed while this function is running. */ void timer_mod_ns(QEMUTimer *ts, int64_t expire_time); @@ -531,6 +545,9 @@ void timer_mod_ns(QEMUTimer *ts, int64_t expire_time); * * Modify a timer to expiry at @expire_time, taking into * account the scale associated with the timer. + * + * This function is thread-safe but the timer and its timer list must not be + * freed while this function is running. */ void timer_mod(QEMUTimer *ts, int64_t expire_timer); diff --git a/qemu-timer.c b/qemu-timer.c index ed3fcb2..e504747 100644 --- a/qemu-timer.c +++ b/qemu-timer.c @@ -66,6 +66,7 @@ QEMUClock qemu_clocks[QEMU_CLOCK_MAX]; struct QEMUTimerList { QEMUClock *clock; +QemuMutex active_timers_lock; QEMUTimer *active_timers; QLIST_ENTRY(QEMUTimerList) list; QEMUTimerListNotifyCB *notify_cb; @@ -101,6 +102,7 @@ QEMUTimerList *timerlist_new(QEMUClockType type, timer_list-clock = clock; timer_list-notify_cb = cb; timer_list-notify_opaque = opaque; +qemu_mutex_init(timer_list-active_timers_lock); QLIST_INSERT_HEAD(clock-timerlists, timer_list, list); return timer_list; } @@ -111,6 +113,7 @@ void timerlist_free(QEMUTimerList *timer_list) if (timer_list-clock) { QLIST_REMOVE(timer_list, list); } +qemu_mutex_destroy(timer_list-active_timers_lock); g_free(timer_list); } @@ -163,9 +166,17 @@ bool qemu_clock_has_timers(QEMUClockType type) bool timerlist_expired(QEMUTimerList *timer_list) { -return (timer_list-active_timers -timer_list-active_timers-expire_time -qemu_clock_get_ns(timer_list-clock-type)); +int64_t expire_time; + +qemu_mutex_lock(timer_list-active_timers_lock); +if (!timer_list-active_timers) { +
Re: [Qemu-devel] [RFC V8 03/13] quorum: Add quorum_aio_writev and its dependencies.
Le Friday 27 Sep 2013 à 12:03:07 (+0200), Kevin Wolf a écrit : Am 26.09.2013 um 18:29 hat Benoît Canet geschrieben: Le Friday 08 Feb 2013 à 11:38:38 (+0100), Kevin Wolf a écrit : Am 28.01.2013 18:07, schrieb Benoît Canet: Signed-off-by: Benoit Canet ben...@irqsave.net --- block/quorum.c | 111 1 file changed, 111 insertions(+) diff --git a/block/quorum.c b/block/quorum.c index d8fffbe..5d8470b 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -52,11 +52,122 @@ struct QuorumAIOCB { int vote_ret; }; +static void quorum_aio_cancel(BlockDriverAIOCB *blockacb) +{ +QuorumAIOCB *acb = container_of(blockacb, QuorumAIOCB, common); +bool finished = false; + +/* Wait for the request to finish */ +acb-finished = finished; +while (!finished) { +qemu_aio_wait(); +} +} + +static AIOCBInfo quorum_aiocb_info = { +.aiocb_size = sizeof(QuorumAIOCB), +.cancel = quorum_aio_cancel, +}; + +static void quorum_aio_bh(void *opaque) +{ +QuorumAIOCB *acb = opaque; +BDRVQuorumState *s = acb-bqs; +int ret; + +ret = s-threshold = acb-success_count ? 0 : -EIO; It would be very much preferable if you stored the actual error code instead of turning everything into -EIO. + +qemu_bh_delete(acb-bh); +acb-common.cb(acb-common.opaque, ret); +if (acb-finished) { +*acb-finished = true; +} +g_free(acb-aios); +qemu_aio_release(acb); +} Move this down so that it's next to the function using the bottom half. + +static QuorumAIOCB *quorum_aio_get(BDRVQuorumState *s, + BlockDriverState *bs, + QEMUIOVector *qiov, + uint64_t sector_num, + int nb_sectors, + BlockDriverCompletionFunc *cb, + void *opaque) +{ +QuorumAIOCB *acb = qemu_aio_get(quorum_aiocb_info, bs, cb, opaque); +int i; + +acb-aios = g_new0(QuorumSingleAIOCB, s-total); + +acb-bqs = s; +acb-qiov = qiov; +acb-bh = NULL; +acb-count = 0; +acb-success_count = 0; +acb-sector_num = sector_num; +acb-nb_sectors = nb_sectors; +acb-vote = NULL; +acb-vote_ret = 0; +acb-finished = NULL; + +for (i = 0; i s-total; i++) { +acb-aios[i].buf = NULL; +acb-aios[i].ret = 0; +acb-aios[i].parent = acb; +} Would you mind to reorder the initialisation of the fields according to the order that is used in the struct definition? + +return acb; +} + +static void quorum_aio_cb(void *opaque, int ret) +{ +QuorumSingleAIOCB *sacb = opaque; +QuorumAIOCB *acb = sacb-parent; +BDRVQuorumState *s = acb-bqs; + +sacb-ret = ret; +acb-count++; +if (ret == 0) { +acb-success_count++; +} +assert(acb-count = s-total); +assert(acb-success_count = s-total); +if (acb-count s-total) { +return; +} + +acb-bh = qemu_bh_new(quorum_aio_bh, acb); +qemu_bh_schedule(acb-bh); What's the reason for using a bottom half here? Worth a comment? multiwrite_cb() in block.c doesn't use one to achieve something similar. Is it buggy when you need one here? I tried the code without bh and it doesn't work. It's long ago tbat I wrote that comment, but the remark about multiwrite_cb() concerns me. Do you know _why_ it doesn't work without the BH, and whether the same problem affects multiwrite_cb()? I'd prefer if we understood what we're doing over just basing the code on experiments. Tried to do the conversion again. It seems to works fine. Best regards Benoît Kevin
Re: [Qemu-devel] mail to Paul Brook p...@codesourcery.com bouncing [postmas...@relay1.mentorg.com: Delivery Status Notification (Failure)]
Am 30.09.2013 12:55, schrieb Michael S. Tsirkin: Anyone knows what's going on? I've already ping'ed him on IRC without success and wrote to another email address that I found on GitHub, where there was some recent activity. If no mail should be sent to p...@codesourcery.com, let's add .mailcap so get_maintainer doesn't suggest this address. I would rather suggest that if someone has not been actively maintaining things for a while and is ignoring requests for what is going on, we should remove that person from MAINTAINERS. Most ARM files won't have recent SoBs from him fixing the larger part of CCs, then .mailcap would be mainly relevant for ColdFire. Regards, Andreas - Forwarded message from postmas...@relay1.mentorg.com - Date: Mon, 30 Sep 2013 03:08:22 -0700 From: postmas...@relay1.mentorg.com To: m...@redhat.com Subject: Delivery Status Notification (Failure) Message-ID: eql24hcd30004d...@svr-orw-fem-01.mgc.mentorg.com This is an automatically generated Delivery Status Notification. Delivery to the following recipients failed. paul_br...@mentor.com Reporting-MTA: dns;svr-orw-fem-01.mgc.mentorg.com Received-From-MTA: dns;relay1.mentorg.com Arrival-Date: Mon, 30 Sep 2013 03:08:21 -0700 Final-Recipient: rfc822;paul_br...@mentor.com Action: failed Status: 5.7.1 X-Display-Name: Brook, Paul Date: Mon, 30 Sep 2013 13:10:35 +0300 From: Michael S. Tsirkin m...@redhat.com To: Marcel Apfelbaum marce...@redhat.com Cc: Paolo Bonzini pbonz...@redhat.com, qemu-devel@nongnu.org qemu-devel@nongnu.org, kra...@redhat.com, p...@codesourcery.com, anth...@codemonkey.ws, afaer...@suse.de, s...@weilnetz.de, peter.crosthwa...@xilinx.com, stefa...@redhat.com, jasow...@redhat.com, dk...@verizon.com, alex.william...@redhat.com Subject: Re: [Qemu-devel] [PATCH 0/3] hw: set irq without selecting INTx pin Message-ID: 20130930101035.gb20...@redhat.com In-Reply-To: 1380534200.3439.8.camel@localhost.localdomain On Mon, Sep 30, 2013 at 12:43:20PM +0300, Marcel Apfelbaum wrote: On Mon, 2013-09-30 at 12:14 +0300, Michael S. Tsirkin wrote: On Mon, Sep 30, 2013 at 11:02:06AM +0200, Paolo Bonzini wrote: Il 30/09/2013 10:58, Michael S. Tsirkin ha scritto: As a next step, can we make pci_set_irq non-inline and make it call pci_irq_handler directly, and get rid of the irq field? What irq field? /* IRQ objects for the INTA-INTD pins. */ qemu_irq *irq; That's still used by devices that use common code for PCI and sysbus versions (e.g. USB OHCI and EHCI). Paolo Well this work wouldn't be complete without addressing them anyway. These devices would have to create their own irq in pci-specific code, along the lines of: This irq field is used also in places where pci_set_irq(PCIDevice dev, level) can't infer the INTx: - PCIExpress: qemu_set_irq(dev-irq[dev-exp.hpev_intx],dev-exp.hpev_notified); Well the spec says, explicitly: 6.7.3.4. Software Notification of Hot-Plug Events ... Note that all other interrupt sources within the same Function will assert the same virtual INTx wire when requesting service. I read this to mean that this is a bug, and it should simply use pci_set_irq like all other devices. - vmxnet3 device: qemu_set_irq(d-irq[int_idx], 1); What approach should be used here? Thanks, Marcel - s-irq = dev-irq[3]; + s-irq = qemu_allocate_irqs(pci_set_irq, dev, 1); If there's more than one device like this, we should add /* Return an irq that calls pci_set_irq internally */ qemu_irq *pci_allocate_irq(PCIDevice *); - End forwarded message - -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH v4 2/3] qemu-timer: make qemu_timer_mod_ns() and qemu_timer_del() thread-safe
On 30 Sep 2013, at 13:45, Mike Day wrote: I've applied this set to Paolo's rcu tree - I see a couple of routines that appear to need the active_timers_lock: (line 137 of qemu-timer.c in my tree) void qemu_clock_notify(QEMUClockType type) { QEMUTimerList *timer_list; QEMUClock *clock = qemu_clock_ptr(type); QLIST_FOREACH(timer_list, clock-timerlists, list) { timerlist_notify(timer_list); } } (line 228 of qemu-timer.c in my tree) int64_t qemu_clock_deadline_ns_all(QEMUClockType type) { int64_t deadline = -1; QEMUTimerList *timer_list; QEMUClock *clock = qemu_clock_ptr(type); QLIST_FOREACH(timer_list, clock-timerlists, list) { deadline = qemu_soonest_timeout(deadline, timerlist_deadline_ns(timer_list)); } return deadline; } I think these functions are always called now with the BQL held, so I wonder if they are good candidates for RCU? These routines iterate through the list of timerlists held by a clock. They do not iterate through the list of active timers in a timer list. I believe the latter is what active_timers_lock protects. The list of timers attached to a clock is only modified when timers are created and deleted which is (currently) under the BQL. -- Alex Bligh
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
On 30.09.2013 21:52, Paolo Bonzini wrote: Il 30/09/2013 13:25, Alexander Graf ha scritto: On 27.09.2013, at 10:06, Alexey Kardashevskiy wrote: To be able to boot on newer hardware that the software support, PowerISA defines a logical PVR, one per every PowerISA specification version from 2.04. This adds the compat option which takes values 205 or 206 and forces QEMU to boot the guest with a logical PVR (CPU_POWERPC_LOGICAL_2_05 or CPU_POWERPC_LOGICAL_2_06). The guest reads the logical PVR value from cpu-version property of a CPU device node. Why is the option under -machine instead of -cpu? Because it is still the same CPU and the guest will still read the real PVR from the hardware (which it may not support but this is why we need compatibility mode). It is possible to run QEMU with -cpu POWER8 with some old distro which does not know about power8, it will switch to power6-compat mode, the the user does yum update (or analog) and then the new kernel (with power8 support) will do H_CAS again and this time raw power8 should be selected. -- With best regards Alexey Kardashevskiy -- icq: 52150396
Re: [Qemu-devel] [RFC V8 06/13] quorum: Add quorum mechanism.
Le Friday 08 Feb 2013 à 13:07:03 (+0100), Kevin Wolf a écrit : Am 28.01.2013 18:07, schrieb Benoît Canet: Use gnutls's SHA-256 to compare versions. Signed-off-by: Benoit Canet ben...@irqsave.net --- block/quorum.c | 303 +++- configure | 22 2 files changed, 324 insertions(+), 1 deletion(-) diff --git a/block/quorum.c b/block/quorum.c index e3c6aad..4c552e4 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -13,8 +13,30 @@ * See the COPYING file in the top-level directory. */ +#include gnutls/gnutls.h +#include gnutls/crypto.h #include block/block_int.h +#define HASH_LENGTH 32 + +typedef union QuorumVoteValue { +char h[HASH_LENGTH]; /* SHA-256 hash */ +unsigned long l; /* simpler hash */ +} QuorumVoteValue; + +typedef struct QuorumVoteItem { +int index; +QLIST_ENTRY(QuorumVoteItem) next; +} QuorumVoteItem; + +typedef struct QuorumVoteVersion { +QuorumVoteValue value; +int index; +int vote_count; +QLIST_HEAD(, QuorumVoteItem) items; +QLIST_ENTRY(QuorumVoteVersion) next; +} QuorumVoteVersion; I wonder if it wouldn't become simpler if you used arrays instead of lists. We know that s-total is the upper limit for entries. I am not sure about this. The voting code is a big part of quorum and currently works fine. Stefan already reviewed it. I don't know if it's wize to throw it for a new one for such a small gain. + typedef struct { BlockDriverState **bs; unsigned long long threshold; @@ -32,6 +54,11 @@ typedef struct QuorumSingleAIOCB { QuorumAIOCB *parent; } QuorumSingleAIOCB; +typedef struct QuorumVotes { +QLIST_HEAD(, QuorumVoteVersion) vote_list; +int (*compare)(QuorumVoteValue *a, QuorumVoteValue *b); +} QuorumVotes; Can this be directly embedded into QuorumAIOCB? compare is always quorum_sha256_compare, so why even have a field? We can still introduce it once we add different options. I introduce another compare function later. + struct QuorumAIOCB { BlockDriverAIOCB common; BDRVQuorumState *bqs; @@ -48,6 +75,8 @@ struct QuorumAIOCB { int success_count; /* number of successfully completed AIOCB */ bool *finished; /* completion signal for cancel */ +QuorumVotes votes; + void (*vote)(QuorumAIOCB *acb); int vote_ret; }; @@ -84,6 +113,11 @@ static void quorum_aio_bh(void *opaque) } qemu_bh_delete(acb-bh); + +if (acb-vote_ret) { +ret = acb-vote_ret; +} + acb-common.cb(acb-common.opaque, ret); if (acb-finished) { *acb-finished = true; @@ -95,6 +129,11 @@ static void quorum_aio_bh(void *opaque) qemu_aio_release(acb); } +static int quorum_sha256_compare(QuorumVoteValue *a, QuorumVoteValue *b) +{ +return memcmp(a, b, HASH_LENGTH); +} Comparing a.h and b.h would be cleaner. + static QuorumAIOCB *quorum_aio_get(BDRVQuorumState *s, BlockDriverState *bs, QEMUIOVector *qiov, @@ -118,6 +157,8 @@ static QuorumAIOCB *quorum_aio_get(BDRVQuorumState *s, acb-vote = NULL; acb-vote_ret = 0; acb-finished = NULL; +acb-votes.compare = quorum_sha256_compare; +QLIST_INIT(acb-votes.vote_list); for (i = 0; i s-total; i++) { acb-aios[i].buf = NULL; @@ -145,10 +186,268 @@ static void quorum_aio_cb(void *opaque, int ret) return; } +/* Do the vote */ +if (acb-vote) { +acb-vote(acb); +} This is NULL for all writes and quorum_vote for all reads. Is there any chance that more options will be introduced? If not, why not have a bool is_read and directly call the function here? + acb-bh = qemu_bh_new(quorum_aio_bh, acb); qemu_bh_schedule(acb-bh); } +static void quorum_print_bad(QuorumAIOCB *acb, const char *filename) +{ +fprintf(stderr, quorum: corrected error in quorum file %s: sector_num=% +PRId64 nb_sectors=%i\n, filename, acb-sector_num, +acb-nb_sectors); +} + +static void quorum_print_failure(QuorumAIOCB *acb) +{ +fprintf(stderr, quorum: failure sector_num=% PRId64 nb_sectors=%i\n, +acb-sector_num, acb-nb_sectors); +} + +static void quorum_print_bad_versions(QuorumAIOCB *acb, + QuorumVoteValue *value) +{ +QuorumVoteVersion *version; +QuorumVoteItem *item; +BDRVQuorumState *s = acb-bqs; + +QLIST_FOREACH(version, acb-votes.vote_list, next) { +if (!acb-votes.compare(version-value, value)) { +continue; +} +QLIST_FOREACH(item, version-items, next) { +
Re: [Qemu-devel] [PATCH v6 01/26] qemu: add Error to typedefs
On Sun, 29 Sep 2013 13:58:24 +0300 Michael S. Tsirkin m...@redhat.com wrote: This is so qom headers can use it without pulling in extra headers. Reviewed-by: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- include/qemu/typedefs.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h index a4c1b84..46c3599 100644 --- a/include/qemu/typedefs.h +++ b/include/qemu/typedefs.h @@ -7,6 +7,7 @@ typedef struct QEMUTimer QEMUTimer; typedef struct QEMUTimerListGroup QEMUTimerListGroup; typedef struct QEMUFile QEMUFile; typedef struct QEMUBH QEMUBH; +typedef struct Error Error; typedef struct AioContext AioContext; fc19 seems to ok with it but rhel6/gcc-4.4.7-4 isn't ./configure --enable-kvm --target-list=x86_64-softmmu,x86_64-linux-user In file included from /qemu/include/qapi/visitor.h:18, from qapi-visit.h:20, from qapi-visit.c:18: /qemu/include/qapi/error.h:23: error: redefinition of typedef ‘Error’ /qemu/include/qemu/typedefs.h:10: note: previous declaration of ‘Error’ was here
Re: [Qemu-devel] [PATCH v4 2/3] qemu-timer: make qemu_timer_mod_ns() and qemu_timer_del() thread-safe
On Mon, Sep 30, 2013 at 8:55 AM, Alex Bligh a...@alex.org.uk wrote: On 30 Sep 2013, at 13:45, Mike Day wrote: I've applied this set to Paolo's rcu tree - I see a couple of routines that appear to need the active_timers_lock: (line 137 of qemu-timer.c in my tree) void qemu_clock_notify(QEMUClockType type) { QEMUTimerList *timer_list; QEMUClock *clock = qemu_clock_ptr(type); QLIST_FOREACH(timer_list, clock-timerlists, list) { timerlist_notify(timer_list); } } (line 228 of qemu-timer.c in my tree) int64_t qemu_clock_deadline_ns_all(QEMUClockType type) { int64_t deadline = -1; QEMUTimerList *timer_list; QEMUClock *clock = qemu_clock_ptr(type); QLIST_FOREACH(timer_list, clock-timerlists, list) { deadline = qemu_soonest_timeout(deadline, timerlist_deadline_ns(timer_list)); } return deadline; } I think these functions are always called now with the BQL held, so I wonder if they are good candidates for RCU? These routines iterate through the list of timerlists held by a clock. They do not iterate through the list of active timers in a timer list. I believe the latter is what active_timers_lock protects. The list of timers attached to a clock is only modified when timers are created and deleted which is (currently) under the BQL. Sorry, and thanks for the correction re: active_timers_lock. I should have said that clock-timerlists may need its own mutex if and when we remove the BQL from the timer code. Mike
Re: [Qemu-devel] [PATCH] spapr: add compat machine option
On 30.09.2013 21:25, Alexander Graf wrote: On 27.09.2013, at 10:06, Alexey Kardashevskiy wrote: To be able to boot on newer hardware that the software support, PowerISA defines a logical PVR, one per every PowerISA specification version from 2.04. This adds the compat option which takes values 205 or 206 and forces QEMU to boot the guest with a logical PVR (CPU_POWERPC_LOGICAL_2_05 or CPU_POWERPC_LOGICAL_2_06). The guest reads the logical PVR value from cpu-version property of a CPU device node. Cc: Nikunj A Dadhania nik...@linux.vnet.ibm.com Cc: Andreas Färber afaer...@suse.de Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru --- hw/ppc/spapr.c | 40 include/hw/ppc/spapr.h | 2 ++ target-ppc/cpu-models.h | 10 ++ target-ppc/cpu.h| 3 +++ target-ppc/kvm.c| 2 ++ vl.c| 4 6 files changed, 61 insertions(+) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index a09a1d9..737452d 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -33,6 +33,7 @@ #include sysemu/kvm.h #include kvm_ppc.h #include mmu-hash64.h +#include cpu-models.h #include hw/boards.h #include hw/ppc/ppc.h @@ -196,6 +197,26 @@ static XICSState *xics_system_init(int nr_servers, int nr_irqs) return icp; } +static void spapr_compat_mode_init(sPAPREnvironment *spapr) +{ +QemuOpts *machine_opts = qemu_get_machine_opts(); +uint64_t compat = qemu_opt_get_number(machine_opts, compat, 0); + +switch (compat) { +case 0: +break; +case 205: +spapr-arch_compat = CPU_POWERPC_LOGICAL_2_05; +break; +case 206: +spapr-arch_compat = CPU_POWERPC_LOGICAL_2_06; Does it make sense to declare compat mode a number or would a string be easier for users? I can imagine that -machine compat=power6 is easier to understand for a user than -machine compat=205. I just follow the PowerISA spec. It does not say anywhere (at least I do not see it) that 2.05==power6. 2.05 was released when power6 was released and power6 supports 2.05 but these are not synonims. And compat=power6 would not set cpu-version to any of power6 PVRs, it still will be a logical PVR. It confuses me too to tell qemu 205 instead of power6 but it is the spec to blame :) +break; +default: +perror(Unsupported mode, only are 205, 206 supported\n); +break; +} +} + static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr) { int ret = 0, offset; @@ -206,6 +227,7 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr) CPU_FOREACH(cpu) { DeviceClass *dc = DEVICE_GET_CLASS(cpu); +CPUPPCState *env = (POWERPC_CPU(cpu)-env); uint32_t associativity[] = {cpu_to_be32(0x5), cpu_to_be32(0x0), cpu_to_be32(0x0), @@ -238,6 +260,14 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr) if (ret 0) { return ret; } + +if (env-arch_compat) { +ret = fdt_setprop(fdt, offset, cpu-version, + env-arch_compat, sizeof(env-arch_compat)); +if (ret 0) { +return ret; +} +} } return ret; } @@ -1145,6 +1175,8 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args) spapr = g_malloc0(sizeof(*spapr)); QLIST_INIT(spapr-phbs); +spapr_compat_mode_init(spapr); + cpu_ppc_hypercall = emulate_spapr_hypercall; /* Allocate RMA if necessary */ @@ -1226,6 +1258,14 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args) xics_cpu_setup(spapr-icp, cpu); +/* + * If compat mode is set in the command line, pass it to CPU so KVM + * will be able to set it in the host kernel. + */ +if (spapr-arch_compat) { +env-arch_compat = spapr-arch_compat; You should set the compat mode in KVM here, rather than doing it in the put_registers call which gets invoked on every register sync. Or can the guest change the mode? I will change it here in the next patch (which requires kernel changes which are not there yet). The guest cannot change it directly but it can indirectly via client-architecture-support. Also, we need to handle failure. If the kernel can not set the CPU to 2.05 mode for example (IIRC POWER8 doesn't allow you to) we should bail out here. Yep, I'll add this easy check :) And then there's the TCG question. We either have to disable CPU features similar to how we handle it in KVM (by setting and honoring the respective bits in PCR) or we need to bail out too and declare compat mode unsupported for TCG. At the moment we want to run old distro on new CPUs. This patch would make more sense with the ibm,client-architecture-support and power8 registers migration patches which I did not post yet.
Re: [Qemu-devel] [PATCH v4 2/3] qemu-timer: make qemu_timer_mod_ns() and qemu_timer_del() thread-safe
Mike, void qemu_clock_notify(QEMUClockType type) ... int64_t qemu_clock_deadline_ns_all(QEMUClockType type) ... I think these functions are always called now with the BQL held, so I wonder if they are good candidates for RCU? These routines iterate through the list of timerlists held by a clock. They do not iterate through the list of active timers in a timer list. I believe the latter is what active_timers_lock protects. The list of timers attached to a clock is only modified when timers are created and deleted which is (currently) under the BQL. Sorry, and thanks for the correction re: active_timers_lock. I should have said that clock-timerlists may need its own mutex if and when we remove the BQL from the timer code. Correct. The list of timerlists is only modified when a QEMUTimerListGroup is created or destroyed (currently when an AioContext is created or destroyed), and that is done with the BQL held. As you point out, it's currently walked by a couple of functions that also have the BQL held. I think the most likely change here is that the walkers might move outside the BQL. Given modification of this list is so rare, the lock would be very very read heavy, so RCU is probably a sensible option. This link may (or may not) help in understanding: http://blog.alex.org.uk/2013/08/24/changes-to-qemus-timer-system/ -- Alex Bligh
Re: [Qemu-devel] Minimal Qemu build for Plan9
Hi, Am 30.09.2013 10:00, schrieb Ashish Kaila: I am a graduate student from CMU and currently working on trying to port Qemu on plan9. I was trying to initially build a minimal Qemu with the least set of devices that are necessary to support a machine, however I noticed that I couldn’t understand some of the dependencies such as requiring an audio driver during initialization. I am currently trying to port just the i386-softmmu version of Qemu to plan9 to keep things simple, here I noticed that pc_init1() initializes a host of devices on the PCI bus like the NIC card etc and was wondering if there is a bare minimum set of devices that Qemu needs to be built with ? Configuring with --target-list=i386-softmmu should already give you a minimal set of devices, omitting those that are irrelevant for x86 but including a few optional ones. However I don't understand why devices emulated in C code would pose a porting problem for you? If the code is not using portable POSIX constructs or suitable #ifdefs then please post patches to fix that rather than working around those type of things. Additionally if someone could point me to a to-do list of things to look out for while porting Qemu on a new platform (new OS/hardware), I would be extremely grateful. osdep.c and related files come to mind, configure obviously. Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 1/2] roms: build two seabios binaries
diff --git a/roms/config.seabios-128k b/roms/config.seabios-128k new file mode 100644 index 000..23ca812 --- /dev/null +++ b/roms/config.seabios-128k @@ -0,0 +1,5 @@ +# for qemu machine types 1.6 + older +# need to turn off features (xhci) to make it fit into 128k +CONFIG_QEMU=y +CONFIG_ROM_SIZE=128 +CONFIG_USB_XHCI=n Is it enough to disable Xen or perhaps OHCI? We can try that. I've picked xhci because it is new in master, so we don't take something away. diff --git a/roms/config.seabios-256k b/roms/config.seabios-256k new file mode 100644 index 000..cc37a78 --- /dev/null +++ b/roms/config.seabios-256k @@ -0,0 +1,3 @@ +# for qemu machine types 1.7 + newer +CONFIG_QEMU=y +CONFIG_ROM_SIZE=256 Is there already an option to remove ACPI tables generation? Should it be enabled for 1.7, irrespective of the 128k-256k switch? Not yet, but will be once mst's patches are in. Yes, we can set that for the 256k rom. cheers, Gerd
Re: [Qemu-devel] Minimal Qemu build for Plan9
On 30 September 2013 17:00, Ashish Kaila ashishkai...@gmail.com wrote: Additionally if someone could point me to a to-do list of things to look out for while porting Qemu on a new platform (new OS/hardware), I would be extremely grateful. It doesn't get ported to new OSes very often, so there's no todo list already written. Do make sure you're using either a reasonably recent (4.x) gcc or clang: we make use of various compiler extensions that I don't think any other C compiler provides. Looking at the code I don't think we require an audio driver: if there is no available audio backend for the platform we should fall back to the two work-anywhere backends (discard all audio and send audio to a .wav file). -- PMM
Re: [Qemu-devel] [PATCH 0/2] make -kernel optional for all ARM boards
Am 24.09.2013 02:01, schrieb Peter Maydell: Make -kernel optional for all ARM boards, by allowing arm_load_kernel() to return without doing anything if no kernel filename was provided. In this case we will start execution at address zero (as the hardware does). This allows the user to provide a flash image or similar on some boards; in particular it's possible to run a UEFI image on the versatile express boards. The second patch tidies up a few boards which had already effectively provided this flexibility at the board level. Peter Maydell (2): hw/arm/boot: Make user not specifying a kernel not an error hw/arm: Tidy up conditional calls to arm_load_kernel Thanks, applied to qom-next: https://github.com/afaerber/qemu-cpu/commits/qom-next Andreas hw/arm/boot.c | 6 -- hw/arm/omap_sx1.c | 10 -- hw/arm/palm.c | 10 -- hw/arm/z2.c | 12 +--- 4 files changed, 17 insertions(+), 21 deletions(-) -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH v3 02/18] arm/boot: Turn arm_load_kernel() into no-op for qtest without -kernel
Am 24.09.2013 01:55, schrieb Peter Maydell: On 23 September 2013 23:51, Andreas Färber afaer...@suse.de wrote: Am 23.09.2013 15:35, schrieb Andreas Färber: Signed-off-by: Andreas Färber afaer...@suse.de --- hw/arm/boot.c | 4 1 file changed, 4 insertions(+) Sorry, I forgot that Grant Likely had an alternative patch [1] not restricted to qtest. Last thing I read was that PMM had similar/further patches. How to proceed? Whoops, I forgot about that. I'll stick the patches I have out on the list, but I'm not really in a position to put them in the arm tree for a pullreq yet; so I'm happy for you to either stick them in this series, Done, thanks. Andreas or to commit this patch (and then I'll fix it up later). -- PMM -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 1/2] roms: build two seabios binaries
Il 30/09/2013 15:44, Gerd Hoffmann ha scritto: +# for qemu machine types 1.6 + older +# need to turn off features (xhci) to make it fit into 128k +CONFIG_QEMU=y +CONFIG_ROM_SIZE=128 +CONFIG_USB_XHCI=n Is it enough to disable Xen or perhaps OHCI? We can try that. I've picked xhci because it is new in master, so we don't take something away. Ok. But if we can enable XHCI and disable Xen, that'd be a win because Xen anyway embeds SeaBIOS within its own hvmloader firmware. Paolo
Re: [Qemu-devel] [PATCH v3 00/18] qtest: Test all targets
Am 23.09.2013 15:35, schrieb Andreas Färber: Andreas Färber (18): mips_mipssim: Silence BIOS loading warning for qtest arm/boot: Turn arm_load_kernel() into no-op for qtest without -kernel With the exception of this patch for which PMM provided a replacement... puv3: Turn puv3_load_kernel() into a no-op for qtest without -kernel mainstone: Don't enforce use of -pflash for qtest gumstix: Don't enforce use of -pflash for qtest z2: Don't enforce use of -pflash for qtest palm: Don't enforce loading ROM or kernel for qtest omap_sx1: Don't enforce use of kernel or flash for qtest exynos4_boards: Silence lack of -smp 2 warning for qtest armv7m: Don't enforce use of kernel for qtest axis_dev88: Don't enforce use of kernel for qtest mcf5208: Don't enforce use of kernel for qtest an5206: Don't enforce use of kernel for qtest milkymist: Suppress -kernel/-bios/-drive error for qtest shix: Drop debug output shix: Don't require firmware presence for qtest leon3: Don't enforce use of -bios with qtest qtest: Prepare QOM machine tests ... applied to qom-next: https://github.com/afaerber/qemu-cpu/commits/qom-next Thanks, Andreas hw/arm/armv7m.c | 25 ++--- hw/arm/boot.c | 4 + hw/arm/exynos4_boards.c | 3 +- hw/arm/gumstix.c| 11 ++- hw/arm/mainstone.c | 5 +- hw/arm/omap_sx1.c | 3 +- hw/arm/palm.c | 3 +- hw/arm/z2.c | 5 +- hw/block/tc58128.c | 10 +- hw/cris/axis_dev88.c| 11 ++- hw/lm32/milkymist.c | 3 +- hw/m68k/an5206.c| 4 + hw/m68k/mcf5208.c | 4 + hw/mips/mips_mipssim.c | 4 +- hw/sh4/shix.c | 16 +-- hw/sparc/leon3.c| 3 +- hw/unicore32/puv3.c | 4 + tests/Makefile | 26 + tests/qom-test.c| 253 19 files changed, 353 insertions(+), 44 deletions(-) create mode 100644 tests/qom-test.c -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH v4 2/3] qemu-timer: make qemu_timer_mod_ns() and qemu_timer_del() thread-safe
On Mon, Sep 30, 2013 at 9:34 AM, Alex Bligh a...@alex.org.uk wrote: void qemu_clock_notify(QEMUClockType type) ... int64_t qemu_clock_deadline_ns_all(QEMUClockType type) ... I think these functions are always called now with the BQL held, so I wonder if they are good candidates for RCU? These routines iterate through the list of timerlists held by a clock. They do not iterate through the list of active timers in a timer list. I believe the latter is what active_timers_lock protects. The list of timers attached to a clock is only modified when timers are created and deleted which is (currently) under the BQL. Sorry, and thanks for the correction re: active_timers_lock. I should have said that clock-timerlists may need its own mutex if and when we remove the BQL from the timer code. Correct. The list of timerlists is only modified when a QEMUTimerListGroup is created or destroyed (currently when an AioContext is created or destroyed), and that is done with the BQL held. As you point out, it's currently walked by a couple of functions that also have the BQL held. I think the most likely change here is that the walkers might move outside the BQL. Given modification of this list is so rare, the lock would be very very read heavy, so RCU is probably a sensible option. This link may (or may not) help in understanding: http://blog.alex.org.uk/2013/08/24/changes-to-qemus-timer-system/ Alex, thanks for the link - Mike
Re: [Qemu-devel] [PATCH v6 01/26] qemu: add Error to typedefs
On Mon, Sep 30, 2013 at 03:10:56PM +0200, Igor Mammedov wrote: On Sun, 29 Sep 2013 13:58:24 +0300 Michael S. Tsirkin m...@redhat.com wrote: This is so qom headers can use it without pulling in extra headers. Reviewed-by: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- include/qemu/typedefs.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h index a4c1b84..46c3599 100644 --- a/include/qemu/typedefs.h +++ b/include/qemu/typedefs.h @@ -7,6 +7,7 @@ typedef struct QEMUTimer QEMUTimer; typedef struct QEMUTimerListGroup QEMUTimerListGroup; typedef struct QEMUFile QEMUFile; typedef struct QEMUBH QEMUBH; +typedef struct Error Error; typedef struct AioContext AioContext; fc19 seems to ok with it but rhel6/gcc-4.4.7-4 isn't ./configure --enable-kvm --target-list=x86_64-softmmu,x86_64-linux-user In file included from /qemu/include/qapi/visitor.h:18, from qapi-visit.h:20, from qapi-visit.c:18: /qemu/include/qapi/error.h:23: error: redefinition of typedef ‘Error’ /qemu/include/qemu/typedefs.h:10: note: previous declaration of ‘Error’ was here Thanks, I'll move that out of qemu/error.h, applying the following on top should fix it, right? Signed-off-by: Michael S. Tsirkin m...@redhat.com -- diff --git a/include/qapi/error.h b/include/qapi/error.h index 7d4c696..b85e996 100644 --- a/include/qapi/error.h +++ b/include/qapi/error.h @@ -13,14 +13,15 @@ #define ERROR_H #include qemu/compiler.h +#include qemu/typedefs.h #include qapi-types.h #include stdbool.h /** - * A class representing internal errors within QEMU. An error has a ErrorClass + * Error: + * An object representing internal errors within QEMU. An error has a ErrorClass * code and a human message. */ -typedef struct Error Error; /** * Set an indirect pointer to an error given a ErrorClass value and a
Re: [Qemu-devel] drive-backup locks VM if target has issues?
On Mon, Sep 30, 2013 at 3:41 AM, Paolo Bonzini pbonz...@redhat.com wrote: Il 30/09/2013 00:46, Wolfgang Richter ha scritto: All writes to the drive-backup source have to first copy the pre-write data to the target. Thus, drive-backup usually works best if you are using werror=stop on the source. That said, I would have expected the job to be cancelled instead. Looks like there are bugs in the handling of on_target_error. Yes, that makes sense and was what I thought as well: it should have been canceled or ended in some bad state. Instead my VM saw drive write errors and remounted root read-only. Not an issue for real work for me, just meant my benchmark couldn't run. My overall goal is to drop the extra write traffic as early as possible to measure overhead of the drive-backup command in a few different scenarios, thus I was hoping /dev/null would help here. I think you need a null backend instead that drops writes at the QEMU level. Perhaps /dev/zero helps too. Yeah, /dev/zero has the same issue. I could make a null backend, or just make my NBD server drop all the writes. There will be extra overhead from TCP, but it'll be good enough for me to measure (NBD is what I am using as a target eventually anyways). -- Wolf
Re: [Qemu-devel] [PATCH v4 2/7] qmp: add internal sync mode common to mirror_start
On 09/30/2013 06:02 AM, Fam Zheng wrote: This adds a new sync mode common which only copies data that is above the common ancestor of source and target. In general, this could be useful in cases like: base_bs --- common_ancestor --- foo --- bar ---source \ \--- target Where data in foo, bar and source will be copied to target, once such common backing_hd sharing is introduced. For now, we could use a special case: If target is the ancestor of source, like, base_bs --- target --- foo --- bar ---source The data in foo, bar and source will be copied to target, like drive-commit, and when they are synced, the source bs replaces target bs. This is specifically useful for block commit of active layer. This mode is not available (-ENOTSUP) from QMP interface, it is only used internally by block commit code. +++ b/qapi-schema.json @@ -1363,7 +1363,7 @@ # Since: 1.3 ## { 'enum': 'MirrorSyncMode', - 'data': ['top', 'full', 'none'] } + 'data': ['top', 'full', 'none', 'common'] } Is it worth documenting the mode, in order to include a '(since 1.7)' notation, as well as a mention that this mode is not supported via QMP but only exists so that the code generator will support the mode needed internally? Is there any way to refactor things so that you don't have to munge the QAPI just to provide this internal-only mode? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature