Re: [Qemu-devel] [Bug] cirrus_vga: qemu abort at booting when configure vgamem_mb <= 2
Hi, > -Original Message- > From: Gerd Hoffmann [mailto:kra...@redhat.com] > Sent: Friday, May 09, 2014 2:49 PM > To: Gonglei (Arei) > Cc: qemu-devel@nongnu.org > Subject: Re: [Bug] cirrus_vga: qemu abort at booting when configure > vgamem_mb <= 2 > > On Fr, 2014-05-09 at 03:47 +, Gonglei (Arei) wrote: > > Hi, Gerd > > > > The issue consequentially occur, I have tested various qemu versions, > > including the current qemu.git. > > Missing sanity check. It's not valid. We mimic existing hardware here, > and the cirrus card emulated has 4 MB video memory. So ideally we > should just use that and be done with it. Problem is qemu has > historically used 8 or 16 mb vga memory for cirrus, and simply changing > it will break live migration. > > So cirrus should accept 4 MB (correct value), 8 MB and 16 MB (for > backward compatibility) and reject everything else. Patches are > welcome ;) > Okay, I will add the check and post a patch. Thanks! > If you want reduce the qemu memory footprint use stdvga instead which > should handle memory sized from 1 MB to 256 MB just fine. > It's OK. > cheers, > Gerd > Best regards, -Gonglei
Re: [Qemu-devel] [PATCH] spapr: pci: clean msi info when releasing it
On Wed, May 7, 2014 at 3:20 PM, Alexey Kardashevskiy wrote:
> On 05/07/2014 04:51 PM, Liu Ping Fan wrote:
>> In current code, we use phb->msi_table[ndev].nvec to indicate whether
>> this msi entries are used by a device or not. So when unplug a pci
>> device, we should reset nvec to zero.
>>
>> Signed-off-by: Liu Ping Fan
>> ---
>> hw/ppc/spapr_pci.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>> index cbef095..7b1dfe1 100644
>> --- a/hw/ppc/spapr_pci.c
>> +++ b/hw/ppc/spapr_pci.c
>> @@ -316,6 +316,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu,
>> sPAPREnvironment *spapr,
>> rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
>> return;
>> }
>> +phb->msi_table[ndev].nvec = 0;
>> trace_spapr_pci_msi("Released MSIs", ndev, config_addr);
>> rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>> rtas_st(rets, 1, 0);
>
>
> ibm,change-msi is called with 0 to disable MSIs. If later the guest decides
> to reenable MSI on the same device (rmmod + modprobe in the guest can do
> that I suppose), new block will be allocated because of this patch which is
> bad.
>
> And there is no PCI hotplug for SPAPR in upstream QEMU so this patch cannot
> possibly fix it :)
>
I saw your patch [PATCH 0/6] move interrupts from spapr to xics. And
it is great to consider the reclaim of irq. So if I call something to
free the irq after "phb->msi_table[ndev].nvec = 0", can it work ?
Thx,
Fan
>
> --
> Alexey
>
Re: [Qemu-devel] [PATCH 1/5] xen, gfx passthrough: basic graphics passthrough support
Anthony PERARD wrote on 2014-03-22:
> On Fri, Feb 21, 2014 at 02:44:09PM +0800, Yang Zhang wrote:
>> From: Yang Zhang
>>
>> basic gfx passthrough support:
>> - add a vga type for gfx passthrough
>> - retrieve VGA bios from host 0xC, then load it to guest 0xC
>> - register/unregister legacy VGA I/O ports and MMIOs for
>> passthroughed gfx
>>
>> The original patch is from Weidong Han
>>
>> Signed-off-by: Yang Zhang
>> Cc: Weidong Han
Hi all,
Thanks for your comments.
I am sorry for late reply. I have been busy working on other task so that I
have no time to push this patch in the past two months. Now I and Tiejun will
continue working on this and will send out the second version which is modified
according your previous comments. Please help to review the second one again.
>> ---
>> configure|2 +- hw/xen/Makefile.objs |
>>2 +- hw/xen/xen-host-pci-device.c |5 ++
>> hw/xen/xen-host-pci-device.h |1 + hw/xen/xen_pt.c |
>> 10 +++ hw/xen/xen_pt.h |4 + hw/xen/xen_pt_graphics.c
>> | 164 ++ qemu-options.hx
>> |9 +++ vl.c |8 ++ 9 files
>> changed, 203 insertions(+), 2 deletions(-) create mode
>> 100644 hw/xen/xen_pt_graphics.c
>>
>> diff --git a/configure b/configure
>> index 4648117..19525ab 100755
>> --- a/configure
>> +++ b/configure
>> @@ -4608,7 +4608,7 @@ case "$target_name" in
>> if test "$xen" = "yes" -a "$target_softmmu" = "yes" ; then
>>echo "CONFIG_XEN=y" >> $config_target_mak
>>if test "$xen_pci_passthrough" = yes; then
>> -echo "CONFIG_XEN_PCI_PASSTHROUGH=y" >> "$config_target_mak" +
>> echo "CONFIG_XEN_PCI_PASSTHROUGH=y" >>
> "$config_host_mak"
>
> Why do you need to move this option from config_target to config_host?
>
>>fi
>> fi
>> ;;
>> diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs index
>> ce640c6..350d337 100644 --- a/hw/xen/Makefile.objs +++
>> b/hw/xen/Makefile.objs @@ -3,4 +3,4 @@ common-obj-$(CONFIG_XEN_BACKEND)
>> += xen_backend.o xen_devconfig.o
>>
>> obj-$(CONFIG_XEN_I386) += xen_platform.o xen_apic.o xen_pvdevice.o
>> obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
>> -obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o
>> xen_pt_msi.o
>> +obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o
>> +xen_pt_msi.o xen_pt_graphics.o
>> diff --git a/hw/xen/xen-host-pci-device.c
>> b/hw/xen/xen-host-pci-device.c index 743b37b..a54b7de 100644
>> --- a/hw/xen/xen-host-pci-device.c
>> +++ b/hw/xen/xen-host-pci-device.c
>> @@ -376,6 +376,11 @@ int xen_host_pci_device_get(XenHostPCIDevice
>> *d,
> uint16_t domain,
>> goto error;
>> }
>> d->irq = v;
>> +rc = xen_host_pci_get_hex_value(d, "class", &v);
>> +if (rc) {
>> +goto error;
>> +}
>> +d->class_code = v;
>> d->is_virtfn = xen_host_pci_dev_is_virtfn(d);
>>
>> return 0;
>> diff --git a/hw/xen/xen-host-pci-device.h
>> b/hw/xen/xen-host-pci-device.h index c2486f0..f1e1c30 100644
>> --- a/hw/xen/xen-host-pci-device.h
>> +++ b/hw/xen/xen-host-pci-device.h
>> @@ -25,6 +25,7 @@ typedef struct XenHostPCIDevice {
>>
>> uint16_t vendor_id; uint16_t device_id; +uint32_t class_code;
>> int irq;
>>
>> XenHostPCIIORegion io_regions[PCI_NUM_REGIONS - 1]; diff --git
>> a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c index be4220b..5a36902 100644
>> --- a/hw/xen/xen_pt.c
>> +++ b/hw/xen/xen_pt.c
>> @@ -450,6 +450,7 @@ static int
> xen_pt_register_regions(XenPCIPassthroughState *s)
>> d->rom.size, d->rom.base_addr);
>> }
>> +register_vga_regions(d);
>> return 0;
>> }
>> @@ -470,6 +471,8 @@ static void
> xen_pt_unregister_regions(XenPCIPassthroughState *s)
>> if (d->rom.base_addr && d->rom.size) {
>> memory_region_destroy(&s->rom);
>> }
>> +
>> +unregister_vga_regions(d);
>> }
>>
>> /* region mapping */
>> @@ -693,6 +696,13 @@ static int xen_pt_initfn(PCIDevice *d)
>> /* Handle real device's MMIO/PIO BARs */
>> xen_pt_register_regions(s);
>> +/* Setup VGA bios for passthroughed gfx */
>> +if (setup_vga_pt(&s->real_device) < 0) {
>> +XEN_PT_ERR(d, "Setup VGA BIOS of passthroughed gfx failed!\n");
>> +xen_host_pci_device_put(&s->real_device);
>> +return -1;
>> +}
>> +
>> /* reinitialize each config register to be emulated */
>> if (xen_pt_config_init(s)) {
>> XEN_PT_ERR(d, "PCI Config space initialisation failed.\n");
>> diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h index
>> 942dc60..c04bbfd
>> 100644
>> --- a/hw/xen/xen_pt.h
>> +++ b/hw/xen/xen_pt.h
>> @@ -298,5 +298,9 @@ static inline bool
> xen_pt_has_msix_mapping(XenPCIPassthroughState *s, int bar)
>> return s->msix && s->msix->bar_index == bar; }
>> +extern int gfx_passthru;
>> +int register_vga_regions(XenHostP
Re: [Qemu-devel] [PATCH v2] block/raw-posix: Try both FIEMAP and SEEK_HOLE
Il 06/05/2014 21:00, Max Reitz ha scritto:
The current version of raw-posix always uses ioctl(FS_IOC_FIEMAP) if
FIEMAP is available; lseek with SEEK_HOLE/SEEK_DATA are not even
compiled in in this case. However, there may be implementations which
support the latter but not the former (e.g., NFSv4.2) as well as vice
versa.
To cover both cases, always try SEEK_HOLE/SEEK_DATA (as this will
probably be covered by POSIX soon) and if that does not work, fall back
to FIEMAP; and if that does not work either, treat everything as
allocated.
I think it's better to test FIEMAP first; if it's implemented, FIEMAP
it's guaranteed to be accurate, and it can also be ruled out on the
first attempt if it's not implemented.
With SEEK_HOLE/SEEK_DATA, the OS could provide a dummy implementation
that returns no holes: this would mean either always falling back to
FIEMAP on fully-allocated files, or producing inaccurate results for
sparse files on filesystems that do not support SEEK_HOLE/SEEK_DATA.
Paolo
Signed-off-by: Max Reitz
---
v2:
- reworked using static functions [Stefan]
- changed order of FIEMAP and SEEK_HOLE [Eric]
---
block/raw-posix.c | 135 ++
1 file changed, 85 insertions(+), 50 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 3ce026d..1b3900d 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -146,6 +146,12 @@ typedef struct BDRVRawState {
bool has_discard:1;
bool has_write_zeroes:1;
bool discard_zeroes:1;
+#if defined SEEK_HOLE && defined SEEK_DATA
+bool skip_seek_hole;
+#endif
+#ifdef CONFIG_FIEMAP
+bool skip_fiemap;
+#endif
} BDRVRawState;
typedef struct BDRVRawReopenState {
@@ -1272,53 +1278,29 @@ static int raw_create(const char *filename,
QEMUOptionParameter *options,
return result;
}
-/*
- * Returns true iff the specified sector is present in the disk image. Drivers
- * not implementing the functionality are assumed to not support backing files,
- * hence all their sectors are reported as allocated.
- *
- * If 'sector_num' is beyond the end of the disk image the return value is 0
- * and 'pnum' is set to 0.
- *
- * 'pnum' is set to the number of sectors (including and immediately following
- * the specified sector) that are known to be in the same
- * allocated/unallocated state.
- *
- * 'nb_sectors' is the max value 'pnum' should be set to. If nb_sectors goes
- * beyond the end of the disk image it will be clamped.
- */
-static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
-int64_t sector_num,
-int nb_sectors, int *pnum)
+static int64_t try_fiemap(BlockDriverState *bs, off_t start, off_t *data,
+ off_t *hole, int nb_sectors, int *pnum)
{
-off_t start, data, hole;
-int64_t ret;
-
-ret = fd_open(bs);
-if (ret < 0) {
-return ret;
-}
-
-start = sector_num * BDRV_SECTOR_SIZE;
-ret = BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | start;
-
#ifdef CONFIG_FIEMAP
-
BDRVRawState *s = bs->opaque;
+int64_t ret = BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | start;
struct {
struct fiemap fm;
struct fiemap_extent fe;
} f;
+if (s->skip_fiemap) {
+return -ENOTSUP;
+}
+
f.fm.fm_start = start;
f.fm.fm_length = (int64_t)nb_sectors * BDRV_SECTOR_SIZE;
f.fm.fm_flags = 0;
f.fm.fm_extent_count = 1;
f.fm.fm_reserved = 0;
if (ioctl(s->fd, FS_IOC_FIEMAP, &f) == -1) {
-/* Assume everything is allocated. */
-*pnum = nb_sectors;
-return ret;
+s->skip_fiemap = true;
+return -errno;
}
if (f.fm.fm_mapped_extents == 0) {
@@ -1326,44 +1308,97 @@ static int64_t coroutine_fn
raw_co_get_block_status(BlockDriverState *bs,
* f.fm.fm_start + f.fm.fm_length must be clamped to the file size!
*/
off_t length = lseek(s->fd, 0, SEEK_END);
-hole = f.fm.fm_start;
-data = MIN(f.fm.fm_start + f.fm.fm_length, length);
+*hole = f.fm.fm_start;
+*data = MIN(f.fm.fm_start + f.fm.fm_length, length);
} else {
-data = f.fe.fe_logical;
-hole = f.fe.fe_logical + f.fe.fe_length;
+*data = f.fe.fe_logical;
+*hole = f.fe.fe_logical + f.fe.fe_length;
if (f.fe.fe_flags & FIEMAP_EXTENT_UNWRITTEN) {
ret |= BDRV_BLOCK_ZERO;
}
}
-#elif defined SEEK_HOLE && defined SEEK_DATA
+return ret;
+#else
+return -ENOTSUP;
+#endif
+}
+static int64_t try_seek_hole(BlockDriverState *bs, off_t start, off_t *data,
+ off_t *hole, int *pnum)
+{
+#if defined SEEK_HOLE && defined SEEK_DATA
BDRVRawState *s = bs->opaque;
-hole = lseek(s->fd, start, SEEK_HOLE);
-if (hole == -1) {
+if (s->skip_seek_hole) {
+return -ENOTSUP;
+}
+
+*hole = lseek(s->fd, start, SEEK_H
[Qemu-devel] [PATCH] kvm: make one_reg helpers available for everyone
s390x introduced helper functions for getting/setting one_regs with
commit 860643bc. However, nothing about these is s390-specific.
Alexey Kardashevskiy had already posted a general version, so let's
merge the two patches and massage the code a bit.
CC: Alexey Kardashevskiy
Signed-off-by: Cornelia Huck
---
include/sysemu/kvm.h | 20
kvm-all.c| 28
target-s390x/kvm.c | 29 -
trace-events | 6 ++
4 files changed, 50 insertions(+), 33 deletions(-)
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 5ad4e0e..a6c2823 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -383,4 +383,24 @@ void kvm_init_irq_routing(KVMState *s);
* > 0: irq chip was created
*/
int kvm_arch_irqchip_create(KVMState *s);
+
+/**
+ * kvm_set_one_reg - set a register value in KVM via KVM_SET_ONE_REG ioctl
+ * @id: The register ID
+ * @source: The pointer to the value to be set. It must point to a variable
+ * of the correct type/size for the register being accessed.
+ *
+ * Returns: 0 on success, or a negative errno on failure.
+ */
+int kvm_set_one_reg(CPUState *cs, uint64_t id, void *source);
+
+/**
+ * kvm_get_one_reg - get a register value from KVM via KVM_GET_ONE_REG ioctl
+ * @id: The register ID
+ * @target: The pointer where the value is to be stored. It must point to a
+ * variable of the correct type/size for the register being accessed.
+ *
+ * Returns: 0 on success, or a negative errno on failure.
+ */
+int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target);
#endif
diff --git a/kvm-all.c b/kvm-all.c
index 5cb7f26..94520e5 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -2114,3 +2114,31 @@ int kvm_create_device(KVMState *s, uint64_t type, bool
test)
return test ? 0 : create_dev.fd;
}
+
+int kvm_set_one_reg(CPUState *cs, uint64_t id, void *source)
+{
+struct kvm_one_reg reg;
+int r;
+
+reg.id = id;
+reg.addr = (uintptr_t) source;
+r = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
+if (r) {
+trace_kvm_failed_reg_set(id, strerror(r));
+}
+return r;
+}
+
+int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target)
+{
+struct kvm_one_reg reg;
+int r;
+
+reg.id = id;
+reg.addr = (uintptr_t) target;
+r = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
+if (r) {
+trace_kvm_failed_reg_get(id, strerror(r));
+}
+return r;
+}
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index b7b0edc..ba2dffe 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -129,35 +129,6 @@ void kvm_arch_reset_vcpu(CPUState *cpu)
}
}
-static int kvm_set_one_reg(CPUState *cs, uint64_t id, void *source)
-{
-struct kvm_one_reg reg;
-int r;
-
-reg.id = id;
-reg.addr = (uint64_t) source;
-r = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®);
-if (r) {
-trace_kvm_failed_reg_set(id, strerror(errno));
-}
-return r;
-}
-
-static int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target)
-{
-struct kvm_one_reg reg;
-int r;
-
-reg.id = id;
-reg.addr = (uint64_t) target;
-r = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
-if (r) {
-trace_kvm_failed_reg_get(id, strerror(errno));
-}
-return r;
-}
-
-
int kvm_arch_put_registers(CPUState *cs, int level)
{
S390CPU *cpu = S390_CPU(cs);
diff --git a/trace-events b/trace-events
index af4449d..2c5b307 100644
--- a/trace-events
+++ b/trace-events
@@ -1230,6 +1230,8 @@ kvm_run_exit(int cpu_index, uint32_t reason) "cpu_index
%d, reason %d"
kvm_device_ioctl(int fd, int type, void *arg) "dev fd %d, type 0x%x, arg %p"
kvm_failed_spr_set(int str, const char *msg) "Warning: Unable to set SPR %d to
KVM: %s"
kvm_failed_spr_get(int str, const char *msg) "Warning: Unable to retrieve SPR
%d from KVM: %s"
+kvm_failed_reg_get(uint64_t id, const char *msg) "Warning: Unable to retrieve
ONEREG %" PRIu64 " from KVM: %s"
+kvm_failed_reg_set(uint64_t id, const char *msg) "Warning: Unable to set
ONEREG %" PRIu64 " to KVM: %s"
# memory.c
memory_region_ops_read(void *mr, uint64_t addr, uint64_t value, unsigned size)
"mr %p addr %#"PRIx64" value %#"PRIx64" size %u"
@@ -1246,7 +1248,3 @@ xen_pv_mmio_write(uint64_t addr) "WARNING: write to Xen
PV Device MMIO space (ad
# hw/pci/pci_host.c
pci_cfg_read(const char *dev, unsigned devid, unsigned fnid, unsigned offs,
unsigned val) "%s %02u:%u @0x%x -> 0x%x"
pci_cfg_write(const char *dev, unsigned devid, unsigned fnid, unsigned offs,
unsigned val) "%s %02u:%u @0x%x <- 0x%x"
-
-# target-s390/kvm.c
-kvm_failed_reg_get(uint64_t id, const char *msg) "Warning: Unable to retrieve
ONEREG %" PRIu64 " from KVM: %s"
-kvm_failed_reg_set(uint64_t id, const char *msg) "Warning: Unable to set
ONEREG %" PRIu64 " to KVM: %s"
--
1.8.5.5
Re: [Qemu-devel] [Bug 1317090] Re: qemu fails on ELF files with no section headers
Hi Craig, On Wed, May 07, 2014 at 03:53:38PM +0100, Peter Maydell wrote: > On 7 May 2014 15:48, Peter Maydell wrote: > > On 7 May 2014 15:34, Paul Jimenez <1317...@bugs.launchpad.net> wrote: > >> Bug description: > >> Using the latest version of qemu-user-static from trusty, 2.0.0+dfsg- > >> 2ubuntu1. > >> > >> Reported to qemu and patch submitted long ago by the guy who wrote > >> http://www.devttys0.com/2011/12/qemu-vs-sstrip/ > >> but apparently dropped on the floor - at least, I can't find it in any > >> qemu bug tracker anywhere. It's now keeping me from running openwrt > >> binaries under qemu-arm-static (because the openwrt guys strip section > >> headers to save space on their teeny embedded boxes). It's a one-line > >> patch, reproduced here: > >> > >> --- qemu/linux-user/elfload.c 2011-12-02 15:16:07.637541215 -0500 > >> +++ qemu-patched/linux-user/elfload.c 2011-12-02 15:27:24.061522798 > >> -0500 > >> @@ -1068,7 +1068,6 @@ static bool elf_check_ehdr(struct elfhdr > >>return (elf_check_arch(ehdr->e_machine) > >>&& ehdr->e_ehsize == sizeof(struct elfhdr) > >>&& ehdr->e_phentsize == sizeof(struct elf_phdr) > >> -&& ehdr->e_shentsize == sizeof(struct elf_shdr) > >>&& (ehdr->e_type == ET_EXEC || ehdr->e_type == ET_DYN)); > >>} > > > > Yeah; the equivalent kernel code: > > http://lxr.linux.no/#linux+v3.14.3/fs/binfmt_elf.c#L595 > > doesn't check the section header size, and nor should QEMU. > > Original 2011 patch: > http://lists.gnu.org/archive/html/qemu-trivial/2011-12/msg00025.html > (hitting the 'reply' button gets us back the original email > address to fix up the signed-off-by line with, so we can > credit the fix to Craig properly.) Can you resend the patch with your Signed-Off-By: ? Riku
Re: [Qemu-devel] [PATCH] block: Fix open flags with BDRV_O_SNAPSHOT
On Wed, May 07, 2014 at 10:24:21AM +0200, Kevin Wolf wrote: > Perhaps the monitor should be changed to avoid printing so many useless > control characters, then we'd hit the limit less often... > > Stefan, didn't you plan to do something like this? Or was it unrelated? I encountered this when working on readline for qemu-io. It's been a while but I remember it's not a trivial fix, would require reworking the readline or monitor code. Stefan
Re: [Qemu-devel] [RFC] vhost: Can we change synchronize_rcu to call_rcu in vhost_set_memory() in vhost kernel module?
Il 09/05/2014 03:57, Gonglei (Arei) ha scritto: Hi, Vhost devices need to do VHOST_SET_MEM_TABLE ioctl in vhost_dev_start() to tell vhost kernel modules GPA to HVA memory mappings, which consume is expensively. The reason is same as KVM_SET_GSI_ROUTING ioctl. That is, in ioctl processing, kmod and vhost calls synchronize_rcu() to wait for grace period to free old memory. In KVM_SET_GSI_ROUTING case, we cannot simply change synchronize_rcu to call_rcu, since this may leads to DOS attacks if guest VM keeps setting IRQ affinity. In VHOST_SET_MEM_TABLE case, I wonder if we can change synchronize_rcu() to call_rcu(), i.e., is it possible to trigger DOS attack in guest? There are some cases QEMU would do VHOST_SET_MEM_TABLE ioctl, like VM start/reboot/attach vhost devices, and RAM memory regions in system memory address space change. And I'd like to know if guest activities could lead to RAM memory regions change? Yes, for example enabling/disabling PCI BARs would have that effect. Paolo
Re: [Qemu-devel] [PATCH v3 18/25] rbd: use BlockDriverState's AioContext
Il 09/05/2014 02:04, Josh Durgin ha scritto:
On 05/08/2014 07:34 AM, Stefan Hajnoczi wrote:
Drop the assumption that we're using the main AioContext. Convert
qemu_bh_new() to aio_bh_new() and qemu_aio_wait() to aio_poll().
The .bdrv_detach_aio_context() and .bdrv_attach_aio_context() interfaces
are not needed since no fd handlers, timers, or BHs stay registered when
requests have been drained.
Cc: Josh Durgin
Signed-off-by: Stefan Hajnoczi
---
block/rbd.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/block/rbd.c b/block/rbd.c
index dbc79f4..41f7bdc 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -548,7 +548,7 @@ static void qemu_rbd_aio_cancel(BlockDriverAIOCB
*blockacb)
acb->cancelled = 1;
while (acb->status == -EINPROGRESS) {
-qemu_aio_wait();
+aio_poll(bdrv_get_aio_context(acb->common.bs), true);
}
qemu_aio_release(acb);
@@ -581,7 +581,8 @@ static void rbd_finish_aiocb(rbd_completion_t c,
RADOSCB *rcb)
rcb->ret = rbd_aio_get_return_value(c);
rbd_aio_release(c);
-acb->bh = qemu_bh_new(rbd_finish_bh, rcb);
+acb->bh = aio_bh_new(bdrv_get_aio_context(acb->common.bs),
+ rbd_finish_bh, rcb);
qemu_bh_schedule(acb->bh);
}
Assuming bdrv_get_aio_context() continues to be safe to call from a
non-qemu thread:
Reviewed-by: Josh Durgin
bdrv_get_aio_context()/bdrv_set_aio_context() are safe if called from
the AioContext's own iothread. This is the case here, in fact
aio_bh_new has the same requirement (qemu_bh_schedule instead does not).
Paolo
Re: [Qemu-devel] [PATCH] linux-user: Return correct errno for unsupported netlink socket
Hi,
On Mon, May 05, 2014 at 08:04:45PM -0700, Ed Swierk wrote:
> This fixes "Cannot open audit interface - aborting." when the
> EAFNOSUPPORT errno differs between the target and host
> architectures (e.g. mips target and x86_64 host).
Thanks, looks good - applied to linux-user tree.
> Signed-off-by: Ed Swierk
> ---
> linux-user/syscall.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 9864813..271d28a 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1861,7 +1861,7 @@ static abi_long do_socket(int domain, int type, int
> protocol)
> }
>
> if (domain == PF_NETLINK)
> -return -EAFNOSUPPORT; /* do not NETLINK socket connections possible
> */
> +return -TARGET_EAFNOSUPPORT;
> ret = get_errno(socket(domain, type, protocol));
> if (ret >= 0) {
> ret = sock_flags_fixup(ret, target_type);
> --
> 1.8.3.2
>
Re: [Qemu-devel] virtio-serial-pci very expensive during live migration
Il 09/05/2014 02:53, Chris Friesen ha scritto: Turns out I spoke too soon. With the patch applied, it boots, but if I try to do a live migration both the source and destination crash. This happens for both the master branch as well as the stable-1.4 branch. The destination doesn't crash, it simply stops because it got an incomplete migration stream. Thanks for the report, the patch seems correct so it's worthwhile looking at it more closely. If I back out the patch, it works fine. If I leave the patch in and disable kvm acceleration it works fine. Indeed, non-KVM doesn't use ioeventfd at all. Paolo
Re: [Qemu-devel] [RFC] dataplane: IOThreads and writing dataplane-capable code
On Thu, May 08, 2014 at 07:58:11PM +0100, Dr. David Alan Gilbert wrote: > * Stefan Hajnoczi (stefa...@gmail.com) wrote: > > On Thu, May 8, 2014 at 3:44 PM, Dr. David Alan Gilbert > > wrote: > > > * Stefan Hajnoczi (stefa...@redhat.com) wrote: > > > > > > > > > > > >> How to synchronize with an IOThread > > >> --- > > >> AioContext is not thread-safe so some rules must be followed when using > > >> file > > >> descriptors, event notifiers, timers, or BHs across threads: > > >> > > >> 1. AioContext functions can be called safely from file descriptor, event > > >> notifier, timer, or BH callbacks invoked by the AioContext. No locking > > >> is > > >> necessary. > > >> > > >> 2. Other threads wishing to access the AioContext must use > > >> aio_context_acquire()/aio_context_release() for mutual exclusion. Once > > >> the > > >> context is acquired no other thread can access it or run event loop > > >> iterations > > >> in this AioContext. > > >> > > >> aio_context_acquire()/aio_context_release() calls may be nested. This > > >> means you can call them if you're not sure whether #1 applies. > > >> > > >> Side note: the best way to schedule a function call across threads is to > > >> create > > >> a BH in the target AioContext beforehand and then call > > >> qemu_bh_schedule(). No > > >> acquire/release or locking is needed for the qemu_bh_schedule() call. > > >> But be > > >> sure to acquire the AioContext for aio_bh_new() if necessary. > > > > > > How do these IOThreads pause during migration? > > > Are they paused by the 'qemu_mutex_lock_iothread' that the migration > > > thread calls? > > > > Currently the only IOThread user is virtio-blk data-plane. It has a > > VM state change listener registered that will stop using the IOThread > > during migration. > > > > In the future we'll have to do more than that: > > It is possible to suspend all IOThreads simply by looping over > > IOThread objects and calling aio_context_acquire() on their > > AioContext. You can release the AioContexts when you are done. This > > would be suitable for a "stop the world" operation for migration > > hand-over. > > That worries me for two reasons: >1) I'm assuming there is some subtlety so that it doesn't deadlock when > another thread is trying to get a couple of contexts. Only the main loop acquires contexts, that's why there is no lock ordering problem. >2) The migration code that has to pause everything is reasonably time > critical (OK not super critical - but it worries if it gains more than > a few > ms). Doing something to each thread in series where that thread might > have to finish up a transaction sounds like it could add together to be > quite > large. It's no different from today where we need to bdrv_drain_all(); bdrv_flush_all(). That's a synchronous operation that can take a while. > > For smaller one-off operations like block-migration.c it may also make > > sense to acquire/release the AioContext. But that's not necessary > > today since dataplane is disabled during migration. > > I guess it's probably right to hide this behind some interface on the Aio > stuff > that migration can call and it can worry about speed, and locking order etc. > > I also would we end up wanting some IOThreads to continue - e.g. could we be > using > them for transport of the migration stream or are they strictly for the guests > use? IOThreads are just threads running AioContext event loops. They are generic and could be used for stuff I/O intensive stuff like migration or the VNC server. Stefan
Re: [Qemu-devel] [PATCH v3 18/25] rbd: use BlockDriverState's AioContext
On Thu, May 08, 2014 at 05:04:16PM -0700, Josh Durgin wrote:
> On 05/08/2014 07:34 AM, Stefan Hajnoczi wrote:
> >Drop the assumption that we're using the main AioContext. Convert
> >qemu_bh_new() to aio_bh_new() and qemu_aio_wait() to aio_poll().
> >
> >The .bdrv_detach_aio_context() and .bdrv_attach_aio_context() interfaces
> >are not needed since no fd handlers, timers, or BHs stay registered when
> >requests have been drained.
> >
> >Cc: Josh Durgin
> >Signed-off-by: Stefan Hajnoczi
> >---
> > block/rbd.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> >diff --git a/block/rbd.c b/block/rbd.c
> >index dbc79f4..41f7bdc 100644
> >--- a/block/rbd.c
> >+++ b/block/rbd.c
> >@@ -548,7 +548,7 @@ static void qemu_rbd_aio_cancel(BlockDriverAIOCB
> >*blockacb)
> > acb->cancelled = 1;
> >
> > while (acb->status == -EINPROGRESS) {
> >-qemu_aio_wait();
> >+aio_poll(bdrv_get_aio_context(acb->common.bs), true);
> > }
> >
> > qemu_aio_release(acb);
> >@@ -581,7 +581,8 @@ static void rbd_finish_aiocb(rbd_completion_t c, RADOSCB
> >*rcb)
> > rcb->ret = rbd_aio_get_return_value(c);
> > rbd_aio_release(c);
> >
> >-acb->bh = qemu_bh_new(rbd_finish_bh, rcb);
> >+acb->bh = aio_bh_new(bdrv_get_aio_context(acb->common.bs),
> >+ rbd_finish_bh, rcb);
> > qemu_bh_schedule(acb->bh);
> > }
>
> Assuming bdrv_get_aio_context() continues to be safe to call from a
> non-qemu thread:
>
> Reviewed-by: Josh Durgin
Yes, we're running in the AioContext associated with this
BlockDriverState. This effectively means we have a lock on the
BlockDriverState.
Re: [Qemu-devel] [Bug 1317090] Re: qemu fails on ELF files with no section headers
On 9 May 2014 09:14, Riku Voipio wrote: > Hi Craig, > > On Wed, May 07, 2014 at 03:53:38PM +0100, Peter Maydell wrote: >> Original 2011 patch: >> http://lists.gnu.org/archive/html/qemu-trivial/2011-12/msg00025.html > >> (hitting the 'reply' button gets us back the original email >> address to fix up the signed-off-by line with, so we can >> credit the fix to Craig properly.) > > Can you resend the patch with your Signed-Off-By: ? Seems a bit unnecessary to force a resend -- the original has the signoff, it's just the mailing list archive has mangled it, so we can just restore it... thanks -- PMM
Re: [Qemu-devel] [PATCH v3.1 00/31] NUMA series, and hostmem improvements
On Thu, May 08, 2014 at 04:51:56PM +0200, Paolo Bonzini wrote:
> Il 06/05/2014 11:27, Hu Tao ha scritto:
> > This series includes work on QOMifying the memory backends.
> > the idea is to delegate all properties of the memory backend to
> > a new QOM class hierarchy, in which the concrete classes
> > are hostmem-ram and hostmem-file. The backend is passed to the
> > machine via "-numa node,memdev=foo" where "foo" is the id of the
> > backend object.
>
> Hello,
>
> I noticed now that if you have the host-nodes property set Linux
> requires you to set a policy other than "default" too. If you don't,
> the mbind system call fails.
>
> What about squashing something like this?
>
> Paolo
>
> diff --git a/backends/hostmem.c b/backends/hostmem.c
> index d3f8476..a0a3111 100644
> --- a/backends/hostmem.c
> +++ b/backends/hostmem.c
> @@ -299,12 +299,23 @@ host_memory_backend_memory_init(UserCreatable *uc,
> Error **errp)
>
> #ifdef CONFIG_NUMA
> unsigned long maxnode = find_last_bit(backend->host_nodes, MAX_NODES);
> +unsigned policy = backend->policy;
> +
> +/* Linux does not accept MPOL_DEFAULT with nonzero bitmap, but
> + * "-object memory-ram,size=128M,hostnodes=0,policy=bind" is a
> + * bit of a mouthful. So if the host_nodes bitmap is nonzero,
> + * pick the BIND policy.
> + */
> +if (find_first_bit(backend->host_nodes, MAX_NODES) != MAX_NODES &&
> +policy == MPOL_DEFAULT) {
> +policy = MPOL_BIND;
> +}
This only changes the policy applied to memory. But info memdev still
shows the policy as 'default'(the 'default' here is interpreted as
'MPOL_DEFAULT', right?).
So this is related to how we deal with cases where 'policy' is not
specified in qemu:
1. if possible, the policy is set to MPOL_DEFAULT.
2. if host-nodes is not zero, the policy is set to MPOL_BIND.(set
backend->policy too, other than just apply it)
Opinions?
>
> /* This is a workaround for a long standing bug in Linux'
> * mbind implementation, which cuts off the last specified
> * node.
> */
> -if (mbind(ptr, sz, backend->policy, backend->host_nodes, maxnode + 2,
> 0)) {
> +if (mbind(ptr, sz, policy, backend->host_nodes, maxnode + 2, 0)) {
> error_setg_errno(errp, errno,
> "cannot bind memory to host NUMA nodes");
Re: [Qemu-devel] [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
Hi,
> -Original Message-
> From: Ian Campbell [mailto:ian.campb...@citrix.com]
> Sent: Thursday, May 08, 2014 7:22 PM
> To: Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; xen-de...@lists.xen.org; jbeul...@suse.com;
> stefano.stabell...@eu.citrix.com; fabio.fant...@m2r.biz;
> anthony.per...@citrix.com; Huangweidong (C); Hanweidong (Randy); Gaowei
> (UVP)
> Subject: Re: [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods
> for PCIslots that support hotplug by runtime patching
>
> 0On Sun, 2014-05-04 at 17:25 +0800, arei.gong...@huawei.com wrote:
> > From: Gaowei
> >
> > In Xen platform, after using upstream qemu, the all of pci devices will show
> > hotplug in the windows guest. In this situation, the windows guest may occur
> > blue screen when VM' user click the icon of VGA card for trying unplug VGA
> card.
> > However, we don't hope VM's user can do such dangerous operation, and
> showing
> > all pci devices inside the guest OS is unfriendly.
> >
> > This is done by runtime patching:
>
> Which appears to involve an awful lot of jumping through hoops... Please
> can you explain why it is necessary, as opposed to e.g. using a dynamic
> set of SSDTs?
>
Ok, we will delete some pointless description.
> > - Rename _EJ0 methods for PCI slots in DSDT to EJ0_:note that this has
> the
> > same checksum, but is ignored by OSPM.
> > - At compile time, look for these methods in ASL source,find the matching
> AML,
> > and store the offsets of these methods in a table named aml_ej0_data.
> > - At run time, go over aml_ej0_data, check which slots not support hotplug
> and
> > patch the ACPI table, replacing _EJ0 with EJ0_.
> >
> > Signed-off-by: Gaowei
> > Signed-off-by: Gonglei
> > ---
> > v3->v2:
> > reformat on the latest master
> > v2->v1:
> > rework by Jan Beulich's suggestion:
> > - adjust the code style
> >
> > tools/firmware/hvmloader/acpi/Makefile | 44 ++-
> > tools/firmware/hvmloader/acpi/acpi2_0.h| 4 +
> > tools/firmware/hvmloader/acpi/build.c | 22 ++
> > tools/firmware/hvmloader/acpi/dsdt.asl | 1 +
> > tools/firmware/hvmloader/acpi/mk_dsdt.c| 2 +
> > tools/firmware/hvmloader/ovmf.c| 6 +-
> > tools/firmware/hvmloader/rombios.c | 4 +
> > tools/firmware/hvmloader/seabios.c | 8 +
> > tools/firmware/hvmloader/tools/acpi_extract.py | 308
> +
> > .../hvmloader/tools/acpi_extract_preprocess.py | 41 +++
> > 10 files changed, 428 insertions(+), 12 deletions(-)
> > create mode 100644 tools/firmware/hvmloader/tools/acpi_extract.py
> > create mode 100644
> tools/firmware/hvmloader/tools/acpi_extract_preprocess.py
> >
> > diff --git a/tools/firmware/hvmloader/acpi/Makefile
> b/tools/firmware/hvmloader/acpi/Makefile
> > index 2c50851..f79ecc3 100644
> > --- a/tools/firmware/hvmloader/acpi/Makefile
> > +++ b/tools/firmware/hvmloader/acpi/Makefile
> > @@ -24,30 +24,46 @@ OBJS = $(patsubst %.c,%.o,$(C_SRC))
> > CFLAGS += $(CFLAGS_xeninclude)
> >
> > vpath iasl $(PATH)
> > +vpath python $(PATH)
>
> I suppose this is trying to make things get rebuilt when the python
> binary is upgraded, but since almost all the functionality is in library
> and modules this doesn't seem like it will be very effective. (I suppose
> it is for iasl which is a single binary, although even then its a bit
> odd to special case iasl in this way out of all the tools used during
> build)
>
> Also please use $(PYTHON) to invoke the scripts.
>
Accept.
> > +.DELETE_ON_ERROR:$(filter dsdt_%.c,$(C_SRC))
>
> Space after the : please.
>
Agreed.
> > +
> > all: acpi.a
> >
> > ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h: %.h: %.asl iasl
> > iasl -vs -p $* -tc $<
> > - sed -e 's/AmlCode/$*/g' $*.hex >$@
> > + sed -e 's/AmlCode/$*/g' $*.hex >$@.tmp
> > + $(call move-if-changed,$@.tmp $@)
> > rm -f $*.hex $*.aml
> >
> > mk_dsdt: mk_dsdt.c
> > $(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -o $@ mk_dsdt.c
> >
> > dsdt_anycpu_qemu_xen.asl: dsdt.asl mk_dsdt
> > - awk 'NR > 1 {print s} {s=$$0}' $< > $@
> > - ./mk_dsdt --dm-version qemu-xen >> $@
> > + awk 'NR > 1 {print s} {s=$$0}' $< > $@.tmp
> > + sed -i 's/AmlCode/dsdt_anycpu_qemu_xen/g' $@.tmp
>
> What is this sed doing?
>
> > + ./mk_dsdt --dm-version qemu-xen >> $@.tmp
> > + sed -i 's/aml_ej0_name/dsdt_anycpu_qemu_xen_aml_ej0_name/g'
> $@.tmp
> > + sed -i 's/aml_adr_dword/dsdt_anycpu_qemu_xen_aml_adr_dword/g'
> $@.tmp
>
> And these two?
>
> Since $@.tmp is programatically generated by mk_dsdt can it not just Do
> The Right Thing in the first place for all of these?
>
Agreed.
> > + $(call move-if-changed,$@.tmp $@)
> >
> > # NB. awk invocation is a portable alternative to 'head -n -1'
> > dsdt_%cpu.asl: dsdt.asl mk_dsdt
> > - awk 'NR > 1 {print s} {s=$$0}' $< > $@
> > - ./mk_dsdt --maxcpu $* >> $@
> > + awk 'NR > 1 {print s} {s=$$0}' $< > $@.tmp
>
Re: [Qemu-devel] [PATCH] spapr: pci: clean msi info when releasing it
On 05/09/2014 05:04 PM, liu ping fan wrote:
> On Wed, May 7, 2014 at 3:20 PM, Alexey Kardashevskiy wrote:
>> On 05/07/2014 04:51 PM, Liu Ping Fan wrote:
>>> In current code, we use phb->msi_table[ndev].nvec to indicate whether
>>> this msi entries are used by a device or not. So when unplug a pci
>>> device, we should reset nvec to zero.
>>>
>>> Signed-off-by: Liu Ping Fan
>>> ---
>>> hw/ppc/spapr_pci.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>>> index cbef095..7b1dfe1 100644
>>> --- a/hw/ppc/spapr_pci.c
>>> +++ b/hw/ppc/spapr_pci.c
>>> @@ -316,6 +316,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu,
>>> sPAPREnvironment *spapr,
>>> rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
>>> return;
>>> }
>>> +phb->msi_table[ndev].nvec = 0;
>>> trace_spapr_pci_msi("Released MSIs", ndev, config_addr);
>>> rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>>> rtas_st(rets, 1, 0);
>>
>>
>> ibm,change-msi is called with 0 to disable MSIs. If later the guest decides
>> to reenable MSI on the same device (rmmod + modprobe in the guest can do
>> that I suppose), new block will be allocated because of this patch which is
>> bad.
>>
>> And there is no PCI hotplug for SPAPR in upstream QEMU so this patch cannot
>> possibly fix it :)
>>
> I saw your patch [PATCH 0/6] move interrupts from spapr to xics. And
> it is great to consider the reclaim of irq. So if I call something to
> free the irq after "phb->msi_table[ndev].nvec = 0", can it work ?
Yes, then it should work but I am actually planning more than just that so
we will see how it goes.
--
Alexey
[Qemu-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
From: Gonglei
In Xen platform, after using upstream qemu, the all of pci devices
will show hotplug in the windows guest, no matter whether they can
be hotpluged. It is unfriendly. The PCI devices that can not be
hotpluged are hidden by modifing the DSDT entries of PCI slots when runtime.
This is done by:
- Rename _EJ0 methods for PCI slots in DSDT to EJ0_:note that
this has the same checksum, but is ignored by OSPM.
- At compile time, look for these methods in ASL source,
find the matching AML, and store the offsets of these methods
in a table named aml_ej0_data.
- At run time, go over aml_ej0_data, check which slots not support
hotplug and patch the ACPI table, replacing _EJ0 with EJ0_.
Cc: Michael S. Tsirkin
Cc: Johannes Krampf
Cc: Kevin O'Connor
Signed-off-by: Gaowei
Signed-off-by: Gonglei
---
v4->v3:
- adjust the code style by Ian Campbell's suggestion
- get permission of Michael, Johannes and Kevin for two python
scripts being used as "GPLv2 or later"
v3->v2:
- reformat on the latest master
v2->v1:
- adjust the code style by Jan Beulich's suggestion
tools/firmware/hvmloader/acpi/Makefile | 37 ++-
tools/firmware/hvmloader/acpi/acpi2_0.h| 4 +
tools/firmware/hvmloader/acpi/build.c | 22 ++
tools/firmware/hvmloader/acpi/dsdt.asl | 1 +
tools/firmware/hvmloader/acpi/mk_dsdt.c| 20 +-
tools/firmware/hvmloader/ovmf.c| 6 +-
tools/firmware/hvmloader/rombios.c | 4 +
tools/firmware/hvmloader/seabios.c | 8 +
tools/firmware/hvmloader/tools/acpi_extract.py | 308 +
.../hvmloader/tools/acpi_extract_preprocess.py | 41 +++
10 files changed, 437 insertions(+), 14 deletions(-)
create mode 100644 tools/firmware/hvmloader/tools/acpi_extract.py
create mode 100644 tools/firmware/hvmloader/tools/acpi_extract_preprocess.py
diff --git a/tools/firmware/hvmloader/acpi/Makefile
b/tools/firmware/hvmloader/acpi/Makefile
index 2c50851..fad1cf5 100644
--- a/tools/firmware/hvmloader/acpi/Makefile
+++ b/tools/firmware/hvmloader/acpi/Makefile
@@ -24,30 +24,45 @@ OBJS = $(patsubst %.c,%.o,$(C_SRC))
CFLAGS += $(CFLAGS_xeninclude)
vpath iasl $(PATH)
+
+.DELETE_ON_ERROR: $(filter dsdt_%.c,$(C_SRC))
+
all: acpi.a
ssdt_s3.h ssdt_s4.h ssdt_pm.h ssdt_tpm.h: %.h: %.asl iasl
iasl -vs -p $* -tc $<
- sed -e 's/AmlCode/$*/g' $*.hex >$@
+ sed -e 's/AmlCode/$*/g' $*.hex >$@.tmp
+ $(call move-if-changed,$@.tmp $@)
rm -f $*.hex $*.aml
mk_dsdt: mk_dsdt.c
- $(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -o $@ mk_dsdt.c
+ $(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -g -o $@ mk_dsdt.c
dsdt_anycpu_qemu_xen.asl: dsdt.asl mk_dsdt
- awk 'NR > 1 {print s} {s=$$0}' $< > $@
- ./mk_dsdt --dm-version qemu-xen >> $@
+ awk 'NR > 1 {print s} {s=$$0}' $< > $@.tmp
+ sed -i 's/AmlCode/dsdt_anycpu_qemu_xen/g' $@.tmp
+ ./mk_dsdt --dm-version qemu-xen --asl-name dsdt_anycpu_qemu_xen >>$@.tmp
+ $(call move-if-changed,$@.tmp $@)
# NB. awk invocation is a portable alternative to 'head -n -1'
dsdt_%cpu.asl: dsdt.asl mk_dsdt
- awk 'NR > 1 {print s} {s=$$0}' $< > $@
- ./mk_dsdt --maxcpu $* >> $@
+ awk 'NR > 1 {print s} {s=$$0}' $< > $@.tmp
+ sed -i 's/AmlCode/dsdt_$*cpu/g' $@.tmp
+ ./mk_dsdt --maxcpu $* >> $@.tmp
+ $(call move-if-changed,$@.tmp $@)
$(filter dsdt_%.c,$(C_SRC)): %.c: iasl %.asl
- iasl -vs -p $* -tc $*.asl
- sed -e 's/AmlCode/$*/g' $*.hex >$@
- echo "int $*_len=sizeof($*);" >>$@
- rm -f $*.aml $*.hex
+ cpp -P $*.asl > $*.asl.i.orig
+ $(PYTHON) ../tools/acpi_extract_preprocess.py $*.asl.i.orig > $*.asl.i
+ iasl -vs -l -tc -p $* $*.asl.i
+ $(PYTHON) ../tools/acpi_extract.py $*.lst > $@.tmp
+ echo "int $*_len=sizeof($*);" >>$@.tmp
+ if grep -q "$*_aml_ej0_name" $@.tmp; then \
+ echo "int
$*_aml_ej0_name_len=sizeof($*_aml_ej0_name)/sizeof($*_aml_ej0_name[0]);"
>>$@.tmp;fi
+ if grep -q "$*_aml_adr_dword" $@.tmp; then \
+ echo "int
$*_aml_adr_dword_len=sizeof($*_aml_adr_dword)/sizeof($*_aml_adr_dword[0]);"
>>$@.tmp;fi
+ $(call move-if-changed,$@.tmp $@)
+ rm -f $*.aml $*.hex $*.asl.i.orig $*.asl.i $*.lst
iasl:
@echo
@@ -64,7 +79,7 @@ acpi.a: $(OBJS)
clean:
rm -rf *.a *.o $(IASL_VER) $(IASL_VER).tar.gz $(DEPS)
- rm -rf ssdt_*.h dsdt*.c *~ *.aml *.hex mk_dsdt dsdt_*.asl
+ rm -rf ssdt_*.h dsdt*.c *~ *.aml *.hex mk_dsdt dsdt_*.asl *.asl.i
*.asl.i.orig *.lst *.tmp
install: all
diff --git a/tools/firmware/hvmloader/acpi/acpi2_0.h
b/tools/firmware/hvmloader/acpi/acpi2_0.h
index 7b22d80..4ba3957 100644
--- a/tools/firmware/hvmloader/acpi/acpi2_0.h
+++ b/tools/firmware/hvmloader/acpi/acpi2_0.h
@@ -396,6 +396,10 @@ struct acpi_config {
int dsdt_anycpu_len;
unsigned char *dsdt_15cpu;
int
Re: [Qemu-devel] QEMU build broken
Hi, On 8 May 2014 17:54, Peter Maydell wrote: > On 8 May 2014 15:47, Brad Smith wrote: > > The following commit broke the build of QEMU.. > > > > linux-user: remove configure option for setting uname release > > > > > http://git.qemu.org/?p=qemu.git;a=commit;h=e586822a58b6609edb5ea929e8a4aa394d32389f > > > > > http://buildbot.b1-systems.de/qemu/builders/default_openbsd_current/builds/752/steps/compile/logs/stdio > > > > /buildbot-qemu/default_openbsd_current/build/bsd-user/main.c:46:34: > error: > > use of undeclared identifier 'CONFIG_UNAME_RELEASE' > > Ah, bsd-user. Do you actually use it, or is it just > in the default compile that you're running? > One year since last bsd-user specific patch, I take we need a new maintainer for bsd-user? In this case bsd-user makes no use at all of the > qemu_uname_release variable so we should probably > just rip it out (together with the useless command > line argument that lets the user tweak it). > Agreed, this is just copypaste from linux-user. Riku
[Qemu-devel] [BUG] 50MB/min logspam: dma: unregistered DMA channel used nchan=0 dma_pos=0 dma_len=1
Hello,
a Xen-4.1-3 host system filled its log file with the message
dma: unregistered DMA channel used nchan=0 dma_pos=0 dma_len=1
The happened two times was a Windows Server 2003 with GPLPV 0.11.0.372:
one VM was migrated to that host and one was directly started there.
After a restart of the VM the message no longer gets printed.
Looking at both xen-4.1.3/tools/ioemu-qemu-xen/hw/dma.c and
qemu-git/hw/dma/i8257.c the message is printed by dma_phony_handler(),
which is the default is no other transfer_handler is registered.
The function gets run through DMA_run_bh -> DMA_run -> channel_run.
In DMA_run() the code checks both the 'mask' and 'status' registers:
383 if ((0 == (d->mask & mask)) && (0 != (d->status & (mask << 4 {
384 channel_run (icont, ichan);
385 rearm = 1;
386}
So the messages is printed with maximum frequency, as the BH gets
executed each time leading to massive log spam.
Looking further I analyzed the following case, which *might* be responsible:
While 'mask' is included in the VMStateDescription, 'status' is not.
Before the conversion to VMSTATE* the code was commented out from day-one:
512 /* qemu_put_8s (f, &d->status); */
540 /* qemu_get_8s (f, &d->status); */
1. Might it be that 'status' is uninitialized after migration?
2. Has someone other seen that message?
3. Could the rate of the message please be limited? See attached patch.
4. Some other ideas or comments?
Sincerely
Philipp
--
Philipp Hahn
Open Source Software Engineer
Univention GmbH
be open.
Mary-Somerville-Str. 1
D-28359 Bremen
Tel.: +49 421 22232-0
Fax : +49 421 22232-99
h...@univention.de
http://www.univention.de/
Geschäftsführer: Peter H. Ganten
HRB 20755 Amtsgericht Bremen
Steuer-Nr.: 71-597-02876
>From 5e54adc8f53df4b8c8e8b802049964b2054ad829 Mon Sep 17 00:00:00 2001
Message-Id: <5e54adc8f53df4b8c8e8b802049964b2054ad829.1399625721.git.h...@univention.de>
From: Philipp Hahn
Date: Fri, 9 May 2014 10:53:57 +0200
Subject: [PATCH] hw/dma: Print error message only once
Organization: Univention GmbH, Bremen, Germany
To: qemu-devel@nongnu.org
otherwise the message
dma: unregistered DMA channel used nchan=0 dma_pos=0 dma_len=1
gets printed every time and fills up the log-file with 50 MiB / minute.
Signed-off-by: Philipp Hahn
---
hw/dma/i8257.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/hw/dma/i8257.c b/hw/dma/i8257.c
index 4490372..cd710a9 100644
--- a/hw/dma/i8257.c
+++ b/hw/dma/i8257.c
@@ -473,8 +473,14 @@ static void dma_reset(void *opaque)
static int dma_phony_handler (void *opaque, int nchan, int dma_pos, int dma_len)
{
-dolog ("unregistered DMA channel used nchan=%d dma_pos=%d dma_len=%d\n",
- nchan, dma_pos, dma_len);
+static int once = 0;
+int mask = 1 << nchan;
+
+if (0 == (once & mask)) {
+once |= mask;
+dolog ("unregistered DMA channel used nchan=%d dma_pos=%d dma_len=%d\n",
+ nchan, dma_pos, dma_len);
+}
return dma_pos;
}
--
1.9.1
Re: [Qemu-devel] [Bug 1317090] Re: qemu fails on ELF files with no section headers
On Fri, May 09, 2014 at 09:20:54AM +0100, Peter Maydell wrote: > On 9 May 2014 09:14, Riku Voipio wrote: > > Hi Craig, > > > > On Wed, May 07, 2014 at 03:53:38PM +0100, Peter Maydell wrote: > >> Original 2011 patch: > >> http://lists.gnu.org/archive/html/qemu-trivial/2011-12/msg00025.html > > > >> (hitting the 'reply' button gets us back the original email > >> address to fix up the signed-off-by line with, so we can > >> credit the fix to Craig properly.) > > > > Can you resend the patch with your Signed-Off-By: ? > Seems a bit unnecessary to force a resend -- the original > has the signoff, it's just the mailing list archive has mangled > it, so we can just restore it... Right, missed that bit. Reconstructing the patch.. Riku
Re: [Qemu-devel] [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
On Fri, 2014-05-09 at 08:38 +, Gonglei (Arei) wrote: > > > This is done by runtime patching: > > > > Which appears to involve an awful lot of jumping through hoops... Please > > can you explain why it is necessary, as opposed to e.g. using a dynamic > > set of SSDTs? > > > Ok, we will delete some pointless description. Actually, I was asking for more... Why is runtime patching the only option here?
Re: [Qemu-devel] [RFC] vhost: Can we change synchronize_rcu to call_rcu in vhost_set_memory() in vhost kernel module?
Hi, > -Original Message- > From: Paolo Bonzini [mailto:pbonz...@redhat.com] > Sent: Friday, May 09, 2014 4:15 PM > To: Gonglei (Arei); qemu-devel@nongnu.org > Cc: m...@redhat.com; Herongguang (Stephen); Huangweidong (C) > Subject: Re: [RFC] vhost: Can we change synchronize_rcu to call_rcu in > vhost_set_memory() in vhost kernel module? > > Il 09/05/2014 03:57, Gonglei (Arei) ha scritto: > > Hi, > > > > Vhost devices need to do VHOST_SET_MEM_TABLE ioctl in vhost_dev_start() > > to tell vhost kernel modules GPA to HVA memory mappings, which consume is > expensively. > > The reason is same as KVM_SET_GSI_ROUTING ioctl. That is, in ioctl > processing, > > kmod and vhost calls synchronize_rcu() to wait for grace period to free old > memory. > > > > In KVM_SET_GSI_ROUTING case, we cannot simply change synchronize_rcu > to call_rcu, > > since this may leads to DOS attacks if guest VM keeps setting IRQ affinity. > > > > In VHOST_SET_MEM_TABLE case, I wonder if we can change > synchronize_rcu() to call_rcu(), > > i.e., is it possible to trigger DOS attack in guest? There are some cases > > QEMU > would do > > VHOST_SET_MEM_TABLE ioctl, like VM start/reboot/attach vhost devices, > and RAM memory > > regions in system memory address space change. > > > > And I'd like to know if guest activities could lead to RAM memory regions > change? > > Yes, for example enabling/disabling PCI BARs would have that effect. > Yes, but PCI BARs are mapped in PCI hole, and they are not overlapped with ram memory regions, so disable or enable PCI BARs would not change ram MRs' mapping. Since vhost_region_add/vhost_region_del check if changed MemoryRegionSection is ram, so vhost memoey mappings will not be influenced, is this correct? Best regards, -Gonglei
Re: [Qemu-devel] [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
Hi, Ian > -Original Message- > From: Ian Campbell [mailto:ian.campb...@citrix.com] > Sent: Friday, May 09, 2014 5:05 PM > To: Gonglei (Arei) > Cc: qemu-devel@nongnu.org; xen-de...@lists.xen.org; jbeul...@suse.com; > stefano.stabell...@eu.citrix.com; fabio.fant...@m2r.biz; > anthony.per...@citrix.com; Huangweidong (C); Hanweidong (Randy); Gaowei > (UVP) > Subject: Re: [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods > for PCIslots that support hotplug by runtime patching > > On Fri, 2014-05-09 at 08:38 +, Gonglei (Arei) wrote: > > > > This is done by runtime patching: > > > > > > Which appears to involve an awful lot of jumping through hoops... Please > > > can you explain why it is necessary, as opposed to e.g. using a dynamic > > > set of SSDTs? > > > > > Ok, we will delete some pointless description. > > Actually, I was asking for more... Why is runtime patching the only > option here? > Sorry about that, please help us to review the v4, thanks! Best regards, -Gonglei
Re: [Qemu-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
>>> On 09.05.14 at 10:47, wrote: > From: Gonglei > > In Xen platform, after using upstream qemu, the all of pci devices > will show hotplug in the windows guest, no matter whether they can > be hotpluged. It is unfriendly. The PCI devices that can not be > hotpluged are hidden by modifing the DSDT entries of PCI slots when runtime. > > This is done by: > - Rename _EJ0 methods for PCI slots in DSDT to EJ0_:note that >this has the same checksum, but is ignored by OSPM. > - At compile time, look for these methods in ASL source, >find the matching AML, and store the offsets of these methods >in a table named aml_ej0_data. > - At run time, go over aml_ej0_data, check which slots not support >hotplug and patch the ACPI table, replacing _EJ0 with EJ0_. I think you mistakenly sent this to qemu-devel instead of xen-devel. And it also seem pretty pointless to send a v4 without addressing all comments you got on v3. Jan
Re: [Qemu-devel] Question about RAM migration flags
* Peter Lieven (p...@kamp.de) wrote: > Hi, > > while working on ram migration and reading through the code I realized that > qemu does not stop loading a saved VM or rejecting an incoming migration > if there is a flag in the stream that it does not understand. An unknown flag > is simply ignored. > > In the block migration code there is a catch at the end complaining about > unknown flags, but in RAM migration there isn't. > > Is this on purpose or an error? I think it's in error; the code doesn't have much checking. Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
> -Original Message- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: Friday, May 09, 2014 5:36 PM > To: Gonglei (Arei) > Cc: anthony.per...@citrix.com; ian.campb...@citrix.com; > stefano.stabell...@eu.citrix.com; johannes.kra...@googlemail.com; Gaowei > (UVP); Hanweidong (Randy); Huangweidong (C); ke...@koconnor.net; > fabio.fant...@m2r.biz; qemu-devel@nongnu.org; m...@redhat.com > Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods > for PCIslots that support hotplug by runtime patching > > >>> On 09.05.14 at 10:47, wrote: > > From: Gonglei > > > > In Xen platform, after using upstream qemu, the all of pci devices > > will show hotplug in the windows guest, no matter whether they can > > be hotpluged. It is unfriendly. The PCI devices that can not be > > hotpluged are hidden by modifing the DSDT entries of PCI slots when runtime. > > > > This is done by: > > - Rename _EJ0 methods for PCI slots in DSDT to EJ0_:note that > >this has the same checksum, but is ignored by OSPM. > > - At compile time, look for these methods in ASL source, > >find the matching AML, and store the offsets of these methods > >in a table named aml_ej0_data. > > - At run time, go over aml_ej0_data, check which slots not support > >hotplug and patch the ACPI table, replacing _EJ0 with EJ0_. > > I think you mistakenly sent this to qemu-devel instead of xen-devel. Yep, that's my fault. > And it also seem pretty pointless to send a v4 without addressing > all comments you got on v3. > I don't think so. I have absorbed Ian's all suggestion on v3. And for other questions have been answered too, in despite of is me or not. Best regards, -Gonglei
[Qemu-devel] [PATCH 2/5] block: New bdrv_nb_sectors()
A call to retrieve the image size converts between bytes and sectors
several times:
* BlockDriver method bdrv_getlength() returns bytes.
* refresh_total_sectors() converts to sectors, rounding up, and stores
in total_sectors.
* bdrv_getlength() converts total_sectors back to bytes (now rounded
up to a multiple of the sector size).
* Callers wanting sectors rather bytes convert it right back.
Example: bdrv_get_geometry().
bdrv_nb_sectors() provides a way to omit the last two conversions.
It's exactly bdrv_getlength() with the conversion to bytes omitted.
It's functionally like bdrv_get_geometry() without its odd error
handling.
Reimplement bdrv_getlength() and bdrv_get_geometry() on top of
bdrv_nb_sectors().
The next patches will convert some users of bdrv_getlength() to
bdrv_nb_sectors().
Signed-off-by: Markus Armbruster
---
block.c | 29 +++--
include/block/block.h | 1 +
2 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/block.c b/block.c
index b749d31..44e1f57 100644
--- a/block.c
+++ b/block.c
@@ -689,6 +689,7 @@ static int find_image_format(BlockDriverState *bs, const
char *filename,
/**
* Set the current 'total_sectors' value
+ * Return 0 on success, -errno on error.
*/
static int refresh_total_sectors(BlockDriverState *bs, int64_t hint)
{
@@ -3470,11 +3471,12 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState
*bs)
}
/**
- * Length of a file in bytes. Return < 0 if error or unknown.
+ * Return number of sectors on success, -errno on error.
*/
-int64_t bdrv_getlength(BlockDriverState *bs)
+int64_t bdrv_nb_sectors(BlockDriverState *bs)
{
BlockDriver *drv = bs->drv;
+
if (!drv)
return -ENOMEDIUM;
@@ -3484,19 +3486,26 @@ int64_t bdrv_getlength(BlockDriverState *bs)
return ret;
}
}
-return bs->total_sectors * BDRV_SECTOR_SIZE;
+return bs->total_sectors;
+}
+
+/**
+ * Return length in bytes on success, -errno on error.
+ * The length is always a multiple of BDRV_SECTOR_SIZE.
+ */
+int64_t bdrv_getlength(BlockDriverState *bs)
+{
+int64_t ret = bdrv_nb_sectors(bs);
+
+return ret < 0 ? ret : ret * BDRV_SECTOR_SIZE;
}
/* return 0 as number of sectors if no device present or error */
void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr)
{
-int64_t length;
-length = bdrv_getlength(bs);
-if (length < 0)
-length = 0;
-else
-length = length >> BDRV_SECTOR_BITS;
-*nb_sectors_ptr = length;
+int64_t nb_sectors = bdrv_nb_sectors(bs);
+
+*nb_sectors_ptr = nb_sectors < 0 ? 0 : nb_sectors;
}
void bdrv_set_on_error(BlockDriverState *bs, BlockdevOnError on_read_error,
diff --git a/include/block/block.h b/include/block/block.h
index 467fb2b..0438b58 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -252,6 +252,7 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState
*bs,
const char *backing_file);
int bdrv_get_backing_file_depth(BlockDriverState *bs);
int bdrv_truncate(BlockDriverState *bs, int64_t offset);
+int64_t bdrv_nb_sectors(BlockDriverState *bs);
int64_t bdrv_getlength(BlockDriverState *bs);
int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
--
1.8.1.4
[Qemu-devel] [PATCH 0/5] Clean up around bdrv_getlength()
Issues addressed in this series: * BlockDriver method bdrv_getlength() generally returns -errno, but some implementations return -1 instead. Fix them [PATCH 1]. * Frequent conversions between sectors and bytes complicate the code needlessly. Clean up some [PATCH 2+3]. * bdrv_getlength() always returns a multiple of BDRV_SECTOR_SIZE, but some places appear to be confused about that, and align the result up or down. Don't [PATCH 4]. * bdrv_get_geometry() hides errors. Don't use it in places where errors should be detected [PATCH 5]. Issues not addressed: * There are quite a few literals left in the code where BDRV_SECTOR_SIZE, BDRV_SECTOR_BITS or BDRV_SECTOR_MASK should be used instead. * Error handling is missing in places, but it's not always obvious whether errors can actually happen, and if yes, how to handle them. Markus Armbruster (5): raw-posix: Fix raw_getlength() to always return -errno on error block: New bdrv_nb_sectors() block: Use bdrv_nb_sectors() when sectors, not bytes are wanted block: Drop superfluous aligning of bdrv_getlength()'s value block: Avoid bdrv_get_geometry() where errors should be detected block-migration.c | 9 +++-- block.c | 81 block/qapi.c | 14 +--- block/qcow2.c | 3 +- block/raw-posix.c | 28 block/vmdk.c | 5 ++- include/block/block.h | 1 + qemu-img.c| 93 ++- 8 files changed, 147 insertions(+), 87 deletions(-) -- 1.8.1.4
[Qemu-devel] [PATCH 5/5] block: Avoid bdrv_get_geometry() where errors should be detected
bdrv_get_geometry() hides errors. Use bdrv_nb_sectors() or
bdrv_getlength() instead where that's obviously inappropriate.
Signed-off-by: Markus Armbruster
---
block.c | 11 +---
block/qapi.c | 14 +++---
qemu-img.c | 85 +---
3 files changed, 76 insertions(+), 34 deletions(-)
diff --git a/block.c b/block.c
index 89cab7c..c78362e 100644
--- a/block.c
+++ b/block.c
@@ -5431,7 +5431,7 @@ void bdrv_img_create(const char *filename, const char
*fmt,
if (size && size->value.n == -1) {
if (backing_file && backing_file->value.s) {
BlockDriverState *bs;
-uint64_t size;
+int64_t size;
char buf[32];
int back_flags;
@@ -5450,8 +5450,13 @@ void bdrv_img_create(const char *filename, const char
*fmt,
local_err = NULL;
goto out;
}
-bdrv_get_geometry(bs, &size);
-size *= 512;
+size = bdrv_getlength(bs);
+if (size < 0) {
+error_setg_errno(errp, -size, "Could not get size of '%s'",
+ backing_file->value.s);
+bdrv_unref(bs);
+goto out;
+}
snprintf(buf, sizeof(buf), "%" PRId64, size);
set_option_parameter(param, BLOCK_OPT_SIZE, buf);
diff --git a/block/qapi.c b/block/qapi.c
index af11445..bb8c08e 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -164,19 +164,25 @@ void bdrv_query_image_info(BlockDriverState *bs,
ImageInfo **p_info,
Error **errp)
{
-uint64_t total_sectors;
+int64_t size;
const char *backing_filename;
char backing_filename2[1024];
BlockDriverInfo bdi;
int ret;
Error *err = NULL;
-ImageInfo *info = g_new0(ImageInfo, 1);
+ImageInfo *info;
-bdrv_get_geometry(bs, &total_sectors);
+size = bdrv_getlength(bs);
+if (size < 0) {
+error_setg_errno(errp, -size, "Can't get size of device '%s'",
+ bdrv_get_device_name(bs));
+return;
+}
+info = g_new0(ImageInfo, 1);
info->filename= g_strdup(bs->filename);
info->format = g_strdup(bdrv_get_format_name(bs));
-info->virtual_size= total_sectors * 512;
+info->virtual_size= size;
info->actual_size = bdrv_get_allocated_file_size(bs);
info->has_actual_size = info->actual_size >= 0;
if (bdrv_is_encrypted(bs)) {
diff --git a/qemu-img.c b/qemu-img.c
index 73aac0f..6b4804c 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -940,7 +940,6 @@ static int img_compare(int argc, char **argv)
int64_t sector_num = 0;
int64_t nb_sectors;
int c, pnum;
-uint64_t bs_sectors;
uint64_t progress_base;
for (;;) {
@@ -1002,10 +1001,18 @@ static int img_compare(int argc, char **argv)
buf1 = qemu_blockalign(bs1, IO_BUF_SIZE);
buf2 = qemu_blockalign(bs2, IO_BUF_SIZE);
-bdrv_get_geometry(bs1, &bs_sectors);
-total_sectors1 = bs_sectors;
-bdrv_get_geometry(bs2, &bs_sectors);
-total_sectors2 = bs_sectors;
+total_sectors1 = bdrv_nb_sectors(bs1);
+if (total_sectors1 < 0) {
+error_report("Can't get size of %s: %s",
+ filename1, strerror(-total_sectors1));
+goto out;
+}
+total_sectors2 = bdrv_nb_sectors(bs2);
+if (total_sectors2 < 0) {
+error_report("Can't get size of %s: %s",
+ filename2, strerror(-total_sectors2));
+goto out;
+}
total_sectors = MIN(total_sectors1, total_sectors2);
progress_base = MAX(total_sectors1, total_sectors2);
@@ -1167,7 +1174,7 @@ static int img_convert(int argc, char **argv)
BlockDriver *drv, *proto_drv;
BlockDriverState **bs = NULL, *out_bs = NULL;
int64_t total_sectors, nb_sectors, sector_num, bs_offset;
-uint64_t bs_sectors;
+int64_t *bs_sectors = NULL;
uint8_t * buf = NULL;
size_t bufsectors = IO_BUF_SIZE / BDRV_SECTOR_SIZE;
const uint8_t *buf1;
@@ -1307,7 +1314,8 @@ static int img_convert(int argc, char **argv)
qemu_progress_print(0, 100);
-bs = g_malloc0(bs_n * sizeof(BlockDriverState *));
+bs = g_new(BlockDriverState *, bs_n);
+bs_sectors = g_new(int64_t, bs_n);
total_sectors = 0;
for (bs_i = 0; bs_i < bs_n; bs_i++) {
@@ -1321,8 +1329,14 @@ static int img_convert(int argc, char **argv)
ret = -1;
goto out;
}
-bdrv_get_geometry(bs[bs_i], &bs_sectors);
-total_sectors += bs_sectors;
+bs_sectors[bs_i] = bdrv_nb_sectors(bs[bs_i]);
+if (bs_sectors[bs_i] < 0) {
+error_report("Could not get size of %s: %s",
+ argv[optind + bs_i], strerror(-bs_sectors[bs_i]));
+ret = -1;
+goto out;
+}
+total_sectors += bs_sectors[bs_i];
}
if (sn_opts
[Qemu-devel] [PATCH 1/5] raw-posix: Fix raw_getlength() to always return -errno on error
We got a merry mix of -1 and -errno here.
Signed-off-by: Markus Armbruster
---
block/raw-posix.c | 28 ++--
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 3ce026d..9bf06e5 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1094,12 +1094,12 @@ static int64_t raw_getlength(BlockDriverState *bs)
struct stat st;
if (fstat(fd, &st))
-return -1;
+return -errno;
if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) {
struct disklabel dl;
if (ioctl(fd, DIOCGDINFO, &dl))
-return -1;
+return -errno;
return (uint64_t)dl.d_secsize *
dl.d_partitions[DISKPART(st.st_rdev)].p_size;
} else
@@ -1113,7 +1113,7 @@ static int64_t raw_getlength(BlockDriverState *bs)
struct stat st;
if (fstat(fd, &st))
-return -1;
+return -errno;
if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) {
struct dkwedge_info dkw;
@@ -1123,7 +1123,7 @@ static int64_t raw_getlength(BlockDriverState *bs)
struct disklabel dl;
if (ioctl(fd, DIOCGDINFO, &dl))
-return -1;
+return -errno;
return (uint64_t)dl.d_secsize *
dl.d_partitions[DISKPART(st.st_rdev)].p_size;
}
@@ -1136,6 +1136,7 @@ static int64_t raw_getlength(BlockDriverState *bs)
BDRVRawState *s = bs->opaque;
struct dk_minfo minfo;
int ret;
+int64_t size;
ret = fd_open(bs);
if (ret < 0) {
@@ -1154,7 +1155,11 @@ static int64_t raw_getlength(BlockDriverState *bs)
* There are reports that lseek on some devices fails, but
* irc discussion said that contingency on contingency was overkill.
*/
-return lseek(s->fd, 0, SEEK_END);
+size = lseek(s->fd, 0, SEEK_END);
+if (size < 0) {
+return -errno;
+}
+return size;
}
#elif defined(CONFIG_BSD)
static int64_t raw_getlength(BlockDriverState *bs)
@@ -1192,6 +1197,9 @@ again:
size = LONG_LONG_MAX;
#else
size = lseek(fd, 0LL, SEEK_END);
+if (size < 0) {
+return -errno;
+}
#endif
#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
switch(s->type) {
@@ -1208,6 +1216,9 @@ again:
#endif
} else {
size = lseek(fd, 0, SEEK_END);
+if (size < 0) {
+return -errno;
+}
}
return size;
}
@@ -1216,13 +1227,18 @@ static int64_t raw_getlength(BlockDriverState *bs)
{
BDRVRawState *s = bs->opaque;
int ret;
+int64_t size;
ret = fd_open(bs);
if (ret < 0) {
return ret;
}
-return lseek(s->fd, 0, SEEK_END);
+size = lseek(s->fd, 0, SEEK_END);
+if (size < 0) {
+return -errno;
+}
+return size;
}
#endif
--
1.8.1.4
[Qemu-devel] [PATCH 4/5] block: Drop superfluous aligning of bdrv_getlength()'s value
It returns a multiple of the sector size. Signed-off-by: Markus Armbruster --- block.c | 1 - block/qcow2.c | 1 - 2 files changed, 2 deletions(-) diff --git a/block.c b/block.c index 1b99cb1..89cab7c 100644 --- a/block.c +++ b/block.c @@ -1228,7 +1228,6 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs, Error **errp) error_setg_errno(errp, -total_size, "Could not get image size"); goto out; } -total_size &= BDRV_SECTOR_MASK; /* Create the temporary image */ ret = get_tmp_filename(tmp_filename, PATH_MAX + 1); diff --git a/block/qcow2.c b/block/qcow2.c index 98f624c..4af09bd 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1927,7 +1927,6 @@ static int qcow2_write_compressed(BlockDriverState *bs, int64_t sector_num, /* align end of file to a sector boundary to ease reading with sector based I/Os */ cluster_offset = bdrv_getlength(bs->file); -cluster_offset = (cluster_offset + 511) & ~511; bdrv_truncate(bs->file, cluster_offset); return 0; } -- 1.8.1.4
Re: [Qemu-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
>>> On 09.05.14 at 11:45, wrote: >> From: Jan Beulich [mailto:jbeul...@suse.com] >> And it also seem pretty pointless to send a v4 without addressing >> all comments you got on v3. >> > I don't think so. I have absorbed Ian's all suggestion on v3. And for other > questions have been answered too, in despite of is me or not. Did I overlook then your response to him asking for alternatives to the run time patching? Jan
[Qemu-devel] [PATCH 3/5] block: Use bdrv_nb_sectors() when sectors, not bytes are wanted
Instead of bdrv_nb_sectors().
Aside: a few of these callers don't handle errors. I didn't
investigate whether they should.
Signed-off-by: Markus Armbruster
---
block-migration.c | 9 -
block.c | 40 ++--
block/qcow2.c | 2 +-
block/vmdk.c | 5 ++---
qemu-img.c| 8
5 files changed, 29 insertions(+), 35 deletions(-)
diff --git a/block-migration.c b/block-migration.c
index 56951e0..f5bda8f 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -185,7 +185,7 @@ static int bmds_aio_inflight(BlkMigDevState *bmds, int64_t
sector)
{
int64_t chunk = sector / (int64_t)BDRV_SECTORS_PER_DIRTY_CHUNK;
-if ((sector << BDRV_SECTOR_BITS) < bdrv_getlength(bmds->bs)) {
+if (sector < bdrv_nb_sectors(bmds->bs)) {
return !!(bmds->aio_bitmap[chunk / (sizeof(unsigned long) * 8)] &
(1UL << (chunk % (sizeof(unsigned long) * 8;
} else {
@@ -222,8 +222,7 @@ static void alloc_aio_bitmap(BlkMigDevState *bmds)
BlockDriverState *bs = bmds->bs;
int64_t bitmap_size;
-bitmap_size = (bdrv_getlength(bs) >> BDRV_SECTOR_BITS) +
-BDRV_SECTORS_PER_DIRTY_CHUNK * 8 - 1;
+bitmap_size = bdrv_nb_sectors(bs) + BDRV_SECTORS_PER_DIRTY_CHUNK * 8 - 1;
bitmap_size /= BDRV_SECTORS_PER_DIRTY_CHUNK * 8;
bmds->aio_bitmap = g_malloc0(bitmap_size);
@@ -349,7 +348,7 @@ static void init_blk_migration_it(void *opaque,
BlockDriverState *bs)
int64_t sectors;
if (!bdrv_is_read_only(bs)) {
-sectors = bdrv_getlength(bs) >> BDRV_SECTOR_BITS;
+sectors = bdrv_nb_sectors(bs);
if (sectors <= 0) {
return;
}
@@ -797,7 +796,7 @@ static int block_load(QEMUFile *f, void *opaque, int
version_id)
if (bs != bs_prev) {
bs_prev = bs;
-total_sectors = bdrv_getlength(bs) >> BDRV_SECTOR_BITS;
+total_sectors = bdrv_nb_sectors(bs);
if (total_sectors <= 0) {
error_report("Error getting length of block device %s",
device_name);
diff --git a/block.c b/block.c
index 44e1f57..1b99cb1 100644
--- a/block.c
+++ b/block.c
@@ -2784,18 +2784,16 @@ int bdrv_write_zeroes(BlockDriverState *bs, int64_t
sector_num,
*/
int bdrv_make_zero(BlockDriverState *bs, BdrvRequestFlags flags)
{
-int64_t target_size;
-int64_t ret, nb_sectors, sector_num = 0;
+int64_t target_sectors, ret, nb_sectors, sector_num = 0;
int n;
-target_size = bdrv_getlength(bs);
-if (target_size < 0) {
-return target_size;
+target_sectors = bdrv_nb_sectors(bs);
+if (target_sectors < 0) {
+return target_sectors;
}
-target_size /= BDRV_SECTOR_SIZE;
for (;;) {
-nb_sectors = target_size - sector_num;
+nb_sectors = target_sectors - sector_num;
if (nb_sectors <= 0) {
return 0;
}
@@ -3012,15 +3010,14 @@ static int coroutine_fn
bdrv_aligned_preadv(BlockDriverState *bs,
ret = drv->bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
} else {
/* Read zeros after EOF of growable BDSes */
-int64_t len, total_sectors, max_nb_sectors;
+int64_t total_sectors, max_nb_sectors;
-len = bdrv_getlength(bs);
-if (len < 0) {
-ret = len;
+total_sectors = bdrv_nb_sectors(bs);
+if (total_sectors < 0) {
+ret = total_sectors;
goto out;
}
-total_sectors = DIV_ROUND_UP(len, BDRV_SECTOR_SIZE);
max_nb_sectors = ROUND_UP(MAX(0, total_sectors - sector_num),
align >> BDRV_SECTOR_BITS);
if (max_nb_sectors > 0) {
@@ -3848,21 +3845,21 @@ static int64_t coroutine_fn
bdrv_co_get_block_status(BlockDriverState *bs,
int64_t sector_num,
int nb_sectors, int *pnum)
{
-int64_t length;
+int64_t total_sectors;
int64_t n;
int64_t ret, ret2;
-length = bdrv_getlength(bs);
-if (length < 0) {
-return length;
+total_sectors = bdrv_getlength(bs);
+if (total_sectors < 0) {
+return total_sectors;
}
-if (sector_num >= (length >> BDRV_SECTOR_BITS)) {
+if (sector_num >= total_sectors) {
*pnum = 0;
return 0;
}
-n = bs->total_sectors - sector_num;
+n = total_sectors - sector_num;
if (n < nb_sectors) {
nb_sectors = n;
}
@@ -3893,8 +3890,8 @@ static int64_t coroutine_fn
bdrv_co_get_block_status(BlockDriverState *bs,
ret |= BDRV_BLOCK_ZERO;
} else if (bs->backing_hd) {
BlockDriverState *bs2 = bs->backing_hd;
-int64_t length2 = bdrv_getlength(bs2);
-if (length2 >= 0 && sector_num >= (length2 >> BDRV_SECTOR_BITS)) {
+int64_t nb_sectors2 = bdrv_g
Re: [Qemu-devel] [RFC] vhost: Can we change synchronize_rcu to call_rcu in vhost_set_memory() in vhost kernel module?
Il 09/05/2014 11:04, Gonglei (Arei) ha scritto: > Yes, for example enabling/disabling PCI BARs would have that effect. > Yes, but PCI BARs are mapped in PCI hole, and they are not overlapped with ram memory regions, so disable or enable PCI BARs would not change ram MRs' mapping. PCI BARs can be RAM (one special case being the ROM BAR). Paolo Since vhost_region_add/vhost_region_del check if changed MemoryRegionSection is ram, so vhost memoey mappings will not be influenced, is this correct?
Re: [Qemu-devel] QEMU build broken
On 9 May 2014 09:57, Riku Voipio wrote: > On 8 May 2014 17:54, Peter Maydell wrote: >> Ah, bsd-user. Do you actually use it, or is it just >> in the default compile that you're running? > One year since last bsd-user specific patch, I take we need a new > maintainer for bsd-user? Perhaps so. Stacey Son submitted a set of patches to it back in January, but they were a very large series which needed some restructuring to get through code review and I don't think there's been a respin of those. Personally I would like to see it either (a) actively maintained upstream or (b) just removed from the tree; the current situation doesn't seem very useful. thanks -- PMM
Re: [Qemu-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
On Fri, 2014-05-09 at 09:45 +, Gonglei (Arei) wrote: > > And it also seem pretty pointless to send a v4 without addressing > > all comments you got on v3. > > > I don't think so. I have absorbed Ian's all suggestion on v3. And for other > questions have been answered too, in despite of is me or not. Actually you haven't answered "Why is runtime patching the only option here?" which was originally phrased as: > > Which appears to involve an awful lot of jumping through hoops... Please > > can you explain why it is necessary, as opposed to e.g. using a dynamic > > set of SSDTs? On an unrelated note I think the provenance of the python scripts (i.e. where they came from), and in particular the details of their relicensing should be in the main commit message for future reference. Ian.
[Qemu-devel] [PATCH] gluster: Correctly propagate errors when volume isn't accessible
The docs for glfs_init suggest that the function sets errno on every
failure. In fact it doesn't. As other functions such as
qemu_gluster_open() in the gluster block code report their errors based
on this fact we need to make sure that errno is set on each failure.
This fixes a crash of qemu-img/qemu when a gluster brick isn't
accessible from given host while the server serving the volume
description is.
Thread 1 (Thread 0x77fba740 (LWP 203880)):
#0 0x777673f8 in glfs_lseek () from /usr/lib64/libgfapi.so.0
#1 0x55574a68 in qemu_gluster_getlength ()
#2 0x55565742 in refresh_total_sectors ()
#3 0x5556914f in bdrv_open_common ()
#4 0x5556e8e8 in bdrv_open ()
#5 0x5556f02f in bdrv_open_image ()
#6 0x5556e5f6 in bdrv_open ()
#7 0x555c5775 in bdrv_new_open ()
#8 0x555c5b91 in img_info ()
#9 0x762c9c05 in __libc_start_main () from /lib64/libc.so.6
#10 0x555648ad in _start ()
---
Notes:
I'm also going to report a bug in libgfapi too.
block/gluster.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/block/gluster.c b/block/gluster.c
index 8836085..d0726ec 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -207,6 +207,11 @@ static struct glfs *qemu_gluster_init(GlusterConf *gconf,
const char *filename,
"volume=%s image=%s transport=%s", gconf->server,
gconf->port, gconf->volname, gconf->image,
gconf->transport);
+
+/* glfs_init sometimes doesn't set errno although docs suggest that */
+if (errno == 0)
+errno = EINVAL;
+
goto out;
}
return glfs;
@@ -482,7 +487,7 @@ static int qemu_gluster_create(const char *filename,
glfs = qemu_gluster_init(gconf, filename, errp);
if (!glfs) {
-ret = -EINVAL;
+ret = -errno;
goto out;
}
--
1.9.2
Re: [Qemu-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
Hi, First, please forgive me for my bad English. It's so sad. > -Original Message- > From: Ian Campbell [mailto:ian.campb...@citrix.com] > Sent: Friday, May 09, 2014 5:57 PM > To: Gonglei (Arei) > Cc: Jan Beulich; xen-de...@lists.xen.org; anthony.per...@citrix.com; > stefano.stabell...@eu.citrix.com; johannes.kra...@googlemail.com; Gaowei > (UVP); Hanweidong (Randy); Huangweidong (C); ke...@koconnor.net; > fabio.fant...@m2r.biz; qemu-devel@nongnu.org; m...@redhat.com > Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods > for PCIslots that support hotplug by runtime patching > > On Fri, 2014-05-09 at 09:45 +, Gonglei (Arei) wrote: > > > And it also seem pretty pointless to send a v4 without addressing > > > all comments you got on v3. > > > > > I don't think so. I have absorbed Ian's all suggestion on v3. And for other > > questions have been answered too, in despite of is me or not. > > Actually you haven't answered "Why is runtime patching the only > option here?" which was originally phrased as: > > > Which appears to involve an awful lot of jumping through hoops... Please > > > can you explain why it is necessary, as opposed to e.g. using a dynamic > > > set of SSDTs? > Ian, I understand your mean now, which consider our method to address this issue is maybe unnecessary, right? And you suggest us to use a dynamic set of SSDTs. TBH I don't know more about the dynamic SSDTs, if you have any details, tell me please, thanks in advance! > On an unrelated note I think the provenance of the python scripts (i.e. > where they came from), and in particular the details of their > relicensing should be in the main commit message for future reference. > OK. Thanks. Best regards, -Gonglei
[Qemu-devel] [PATCH] cirrus_vga: adding sanity check for vram size
From: Gonglei
when configure a invalid vram size for cirrus card, such as less
2 MB, which will crash qemu. Follow the real hardware, the cirrus
card has 4 MB video memory. Also for backward compatibility, accept
8 MB and 16 MB vram size.
Signed-off-by: Gonglei
---
For isa cirrus vga device, its' init function has been droped at
commit db895a1e6a97e919f9b86d60c969377357b05066. I have no idea how adding
check on isa_cirrus_vga device. Any ideas? Thanks.
hw/display/cirrus_vga.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index d1afc76..5fec068 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -2959,6 +2959,13 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev)
PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
int16_t device_id = pc->device_id;
+ /* follow real hardware, cirrus card emulated has 4 MB video memory.
+ Also accept 8 MB/16 MB for backward compatibility. */
+ if (s->vga.vram_size_mb != 4 || s->vga.vram_size_mb != 8 ||
+ s->vga.vram_size_mb != 16) {
+ error_report("Invalid cirrus_vga ram size '%u'\n",
s->vga.vram_size_mb);
+ return -1;
+ }
/* setup VGA */
vga_common_init(&s->vga, OBJECT(dev), true);
cirrus_init_common(s, OBJECT(dev), device_id, 1, pci_address_space(dev),
--
1.7.12.4
Re: [Qemu-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
On Fri, 2014-05-09 at 10:15 +, Gonglei (Arei) wrote: > Hi, > > First, please forgive me for my bad English. > It's so sad. > > > -Original Message- > > From: Ian Campbell [mailto:ian.campb...@citrix.com] > > Sent: Friday, May 09, 2014 5:57 PM > > To: Gonglei (Arei) > > Cc: Jan Beulich; xen-de...@lists.xen.org; anthony.per...@citrix.com; > > stefano.stabell...@eu.citrix.com; johannes.kra...@googlemail.com; Gaowei > > (UVP); Hanweidong (Randy); Huangweidong (C); ke...@koconnor.net; > > fabio.fant...@m2r.biz; qemu-devel@nongnu.org; m...@redhat.com > > Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods > > for PCIslots that support hotplug by runtime patching > > > > On Fri, 2014-05-09 at 09:45 +, Gonglei (Arei) wrote: > > > > And it also seem pretty pointless to send a v4 without addressing > > > > all comments you got on v3. > > > > > > > I don't think so. I have absorbed Ian's all suggestion on v3. And for > > > other > > > questions have been answered too, in despite of is me or not. > > > > Actually you haven't answered "Why is runtime patching the only > > option here?" which was originally phrased as: > > > > Which appears to involve an awful lot of jumping through hoops... Please > > > > can you explain why it is necessary, as opposed to e.g. using a dynamic > > > > set of SSDTs? > > > Ian, I understand your mean now, which consider our method to address > this issue is maybe unnecessary, right? And you suggest us to use a dynamic > set of SSDTs. Really what I'm asking is what set of constraints and requirements led you to this particular solution. I think the method seems complicated, and I'd therefore like to know why it was preferred over other alternatives, or perhaps why it is the only option. > TBH I don't know more about the dynamic SSDTs, if you have any details, > tell me please, thanks in advance! I'm not an ACPI expert, but AIUI an SSDT is essentially a little piece of DSDT which is grafted onto the main DSDT at runtime by the OSPM. They make it somewhat easier for BIOS (or ACPI table) authors to include or exclude functionality at runtime, perhaps on a physical system in response to a user changing something in the BIOS setup screens. In Xen we appear to use SSDTs for HPET, TPM and S3/S4 functionality, depending on the guest configuration (hvmloader/acpi/build.c:construct_secondary_tables()). Ian.
Re: [Qemu-devel] [PATCH] cirrus_vga: adding sanity check for vram size [checkpatch false positive?]
On Fr, 2014-05-09 at 18:21 +0800, arei.gong...@huawei.com wrote:
> From: Gonglei
>
> when configure a invalid vram size for cirrus card, such as less
> 2 MB, which will crash qemu. Follow the real hardware, the cirrus
> card has 4 MB video memory. Also for backward compatibility, accept
> 8 MB and 16 MB vram size.
Fails checkpatch:
=== checkpatch complains ===
WARNING: suspect code indent for conditional statements (5, 9)
#12: FILE: hw/display/cirrus_vga.c:2964:
+ if (s->vga.vram_size_mb != 4 || s->vga.vram_size_mb != 8 ||
[...]
+ error_report("Invalid cirrus_vga ram size '%u'\n",
s->vga.vram_size_mb);
WARNING: line over 80 characters
#14: FILE: hw/display/cirrus_vga.c:2966:
+ error_report("Invalid cirrus_vga ram size '%u'\n",
s->vga.vram_size_mb);
total: 0 errors, 2 warnings, 13 lines checked
First warning looks like a false positive though.
MAINTAINERS lists blue swirl as checkpatch maintainer, Cc'ing.
Havn't seen him on the list for quite a while though, is that still
up-to-date?
cheers,
Gerd
>
> Signed-off-by: Gonglei
> ---
> For isa cirrus vga device, its' init function has been droped at
> commit db895a1e6a97e919f9b86d60c969377357b05066. I have no idea how adding
> check on isa_cirrus_vga device. Any ideas? Thanks.
>
> hw/display/cirrus_vga.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
> index d1afc76..5fec068 100644
> --- a/hw/display/cirrus_vga.c
> +++ b/hw/display/cirrus_vga.c
> @@ -2959,6 +2959,13 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev)
> PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
> int16_t device_id = pc->device_id;
>
> + /* follow real hardware, cirrus card emulated has 4 MB video memory.
> + Also accept 8 MB/16 MB for backward compatibility. */
> + if (s->vga.vram_size_mb != 4 || s->vga.vram_size_mb != 8 ||
> + s->vga.vram_size_mb != 16) {
> + error_report("Invalid cirrus_vga ram size '%u'\n",
> s->vga.vram_size_mb);
> + return -1;
> + }
> /* setup VGA */
> vga_common_init(&s->vga, OBJECT(dev), true);
> cirrus_init_common(s, OBJECT(dev), device_id, 1, pci_address_space(dev),
Re: [Qemu-devel] [PATCH] cirrus_vga: adding sanity check for vram size [checkpatch false positive?]
Hi,
> -Original Message-
> From: Gerd Hoffmann [mailto:kra...@redhat.com]
> Sent: Friday, May 09, 2014 6:31 PM
> To: Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; afaer...@suse.de; m...@redhat.com;
> pbonz...@redhat.com; Huangweidong (C); Blue Swirl
> Subject: Re: [PATCH] cirrus_vga: adding sanity check for vram size [checkpatch
> false positive?]
>
> On Fr, 2014-05-09 at 18:21 +0800, arei.gong...@huawei.com wrote:
> > From: Gonglei
> >
> > when configure a invalid vram size for cirrus card, such as less
> > 2 MB, which will crash qemu. Follow the real hardware, the cirrus
> > card has 4 MB video memory. Also for backward compatibility, accept
> > 8 MB and 16 MB vram size.
>
> Fails checkpatch:
>
> === checkpatch complains ===
> WARNING: suspect code indent for conditional statements (5, 9)
> #12: FILE: hw/display/cirrus_vga.c:2964:
> + if (s->vga.vram_size_mb != 4 || s->vga.vram_size_mb != 8 ||
> [...]
> + error_report("Invalid cirrus_vga ram size '%u'\n",
> s->vga.vram_size_mb);
>
> WARNING: line over 80 characters
> #14: FILE: hw/display/cirrus_vga.c:2966:
> + error_report("Invalid cirrus_vga ram size '%u'\n",
> s->vga.vram_size_mb);
>
> total: 0 errors, 2 warnings, 13 lines checked
>
Sorry for my negligence. I will post another patch.
BTW, what's your opinion about isa cirrus vga device, Gerd?
Best regards,
-Gonglei
[Qemu-devel] [PATCH] Split ram_save_block
From: "Dr. David Alan Gilbert"
ram_save_block is getting a bit too complicated, and does two separate
things:
1) Finds a page to send
2) Sends the page (dealing with compression etc)
Split into 'ram_save_page' to send the page and deal with compression (2)
Rename remaining function to 'ram_find_and_save_block'
Signed-off-by: Dr. David Alan Gilbert
---
arch_init.c | 141 ++--
1 file changed, 79 insertions(+), 62 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index be743fd..e33d482 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -560,20 +560,93 @@ static void migration_bitmap_sync(void)
}
/*
- * ram_save_block: Writes a page of memory to the stream f
+ * ram_save_page: Send the given page to the stream
+ *
+ * Returns: Number of bytes written.
+ */
+static int ram_save_page(QEMUFile *f, RAMBlock* block, ram_addr_t offset,
+ bool last_stage)
+{
+int bytes_sent;
+int cont;
+ram_addr_t current_addr;
+MemoryRegion *mr = block->mr;
+uint8_t *p;
+int ret;
+bool send_async = true;
+
+cont = (block == last_sent_block) ? RAM_SAVE_FLAG_CONTINUE : 0;
+
+p = memory_region_get_ram_ptr(mr) + offset;
+
+/* In doubt sent page as normal */
+bytes_sent = -1;
+ret = ram_control_save_page(f, block->offset,
+ offset, TARGET_PAGE_SIZE, &bytes_sent);
+
+XBZRLE_cache_lock();
+
+current_addr = block->offset + offset;
+if (ret != RAM_SAVE_CONTROL_NOT_SUPP) {
+if (ret != RAM_SAVE_CONTROL_DELAYED) {
+if (bytes_sent > 0) {
+acct_info.norm_pages++;
+} else if (bytes_sent == 0) {
+acct_info.dup_pages++;
+}
+}
+} else if (is_zero_range(p, TARGET_PAGE_SIZE)) {
+acct_info.dup_pages++;
+bytes_sent = save_block_hdr(f, block, offset, cont,
+RAM_SAVE_FLAG_COMPRESS);
+qemu_put_byte(f, 0);
+bytes_sent++;
+/* Must let xbzrle know, otherwise a previous (now 0'd) cached
+ * page would be stale
+ */
+xbzrle_cache_zero_page(current_addr);
+} else if (!ram_bulk_stage && migrate_use_xbzrle()) {
+bytes_sent = save_xbzrle_page(f, &p, current_addr, block,
+ offset, cont, last_stage);
+if (!last_stage) {
+/* Can't send this cached data async, since the cache page
+ * might get updated before it gets to the wire
+ */
+send_async = false;
+}
+}
+
+/* XBZRLE overflow or normal page */
+if (bytes_sent == -1) {
+bytes_sent = save_block_hdr(f, block, offset, cont,
RAM_SAVE_FLAG_PAGE);
+if (send_async) {
+qemu_put_buffer_async(f, p, TARGET_PAGE_SIZE);
+} else {
+qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
+}
+bytes_sent += TARGET_PAGE_SIZE;
+acct_info.norm_pages++;
+}
+
+XBZRLE_cache_unlock();
+
+return bytes_sent;
+}
+
+/*
+ * ram_find_and_save_block: Finds a page to send and sends it to f
*
* Returns: The number of bytes written.
* 0 means no dirty pages
*/
-static int ram_save_block(QEMUFile *f, bool last_stage)
+static int ram_find_and_save_block(QEMUFile *f, bool last_stage)
{
RAMBlock *block = last_seen_block;
ram_addr_t offset = last_offset;
bool complete_round = false;
int bytes_sent = 0;
MemoryRegion *mr;
-ram_addr_t current_addr;
if (!block)
block = QTAILQ_FIRST(&ram_list.blocks);
@@ -594,64 +667,8 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
ram_bulk_stage = false;
}
} else {
-int ret;
-uint8_t *p;
-bool send_async = true;
-int cont = (block == last_sent_block) ?
-RAM_SAVE_FLAG_CONTINUE : 0;
-
-p = memory_region_get_ram_ptr(mr) + offset;
-
-/* In doubt sent page as normal */
-bytes_sent = -1;
-ret = ram_control_save_page(f, block->offset,
- offset, TARGET_PAGE_SIZE, &bytes_sent);
-
-XBZRLE_cache_lock();
-
-current_addr = block->offset + offset;
-if (ret != RAM_SAVE_CONTROL_NOT_SUPP) {
-if (ret != RAM_SAVE_CONTROL_DELAYED) {
-if (bytes_sent > 0) {
-acct_info.norm_pages++;
-} else if (bytes_sent == 0) {
-acct_info.dup_pages++;
-}
-}
-} else if (is_zero_range(p, TARGET_PAGE_SIZE)) {
-acct_info.dup_pages++;
-bytes_sent = save_block_hdr(f, block, offset, cont,
-RAM_SAVE_FLAG_COMPRESS);
-qemu_put_byte(f, 0);
-bytes_sent++;
-/*
Re: [Qemu-devel] [PATCH] cirrus_vga: adding sanity check for vram size [checkpatch false positive?]
Hi, > BTW, what's your opinion about isa cirrus vga device, Gerd? I'd do the same check there. cheers, Gerd
Re: [Qemu-devel] [PATCH] cirrus_vga: adding sanity check for vram size [checkpatch false positive?]
> -Original Message- > From: Gerd Hoffmann [mailto:kra...@redhat.com] > Sent: Friday, May 09, 2014 6:55 PM > To: Gonglei (Arei) > Cc: qemu-devel@nongnu.org; afaer...@suse.de; m...@redhat.com; > pbonz...@redhat.com; Huangweidong (C); Blue Swirl > Subject: Re: [PATCH] cirrus_vga: adding sanity check for vram size [checkpatch > false positive?] > > Hi, > > > BTW, what's your opinion about isa cirrus vga device, Gerd? > > I'd do the same check there. > But isa_cirrus_vga_realizefn() has no return value for judgment. Adding the isa init function back? Best regards, -Gonglei
[Qemu-devel] [PATCH v2] cirrus_vga: adding sanity check for vram size
From: Gonglei
when configure a invalid vram size for cirrus card, such as less
2 MB, which will crash qemu. Follow the real hardware, the cirrus
card has 4 MB video memory. Also for backward compatibility, accept
8 MB and 16 MB vram size.
Signed-off-by: Gonglei
---
v2: fix checkpatch fails.
"WARNING: suspect code indent for conditional statements (5, 9)"
maybe not a real warning.
hw/display/cirrus_vga.c | 8
1 file changed, 8 insertions(+)
diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index d1afc76..399a2ef 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -2959,6 +2959,14 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev)
PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
int16_t device_id = pc->device_id;
+ /* follow real hardware, cirrus card emulated has 4 MB video memory.
+ Also accept 8 MB/16 MB for backward compatibility. */
+ if (s->vga.vram_size_mb != 4 || s->vga.vram_size_mb != 8 ||
+ s->vga.vram_size_mb != 16) {
+ error_report("Invalid cirrus_vga ram size '%u'\n",
+ s->vga.vram_size_mb);
+ return -1;
+ }
/* setup VGA */
vga_common_init(&s->vga, OBJECT(dev), true);
cirrus_init_common(s, OBJECT(dev), device_id, 1, pci_address_space(dev),
--
1.7.12.4
Re: [Qemu-devel] [PATCH v2] cirrus_vga: adding sanity check for vram size
09.05.2014 15:04, arei.gong...@huawei.com wrote:
> From: Gonglei
>
> when configure a invalid vram size for cirrus card, such as less
> 2 MB, which will crash qemu. Follow the real hardware, the cirrus
> card has 4 MB video memory. Also for backward compatibility, accept
> 8 MB and 16 MB vram size.
>
> Signed-off-by: Gonglei
> ---
> v2: fix checkpatch fails.
> "WARNING: suspect code indent for conditional statements (5, 9)"
> maybe not a real warning.
>
> hw/display/cirrus_vga.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
> index d1afc76..399a2ef 100644
> --- a/hw/display/cirrus_vga.c
> +++ b/hw/display/cirrus_vga.c
> @@ -2959,6 +2959,14 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev)
> PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
> int16_t device_id = pc->device_id;
>
> + /* follow real hardware, cirrus card emulated has 4 MB video memory.
> + Also accept 8 MB/16 MB for backward compatibility. */
> + if (s->vga.vram_size_mb != 4 || s->vga.vram_size_mb != 8 ||
> + s->vga.vram_size_mb != 16) {
This condition will always be true, because a number can't be equal
to 3 _different_ numbers at the same time.
Thanks,
/mjt
Re: [Qemu-devel] [PATCH] cirrus_vga: adding sanity check for vram size
* arei.gong...@huawei.com (arei.gong...@huawei.com) wrote:
> From: Gonglei
>
> when configure a invalid vram size for cirrus card, such as less
> 2 MB, which will crash qemu. Follow the real hardware, the cirrus
> card has 4 MB video memory. Also for backward compatibility, accept
> 8 MB and 16 MB vram size.
virt-manager/libvirt seems to default to 9 MByte of Vram for cirrus,
so this would break a lot of setups.
Looking at datasheets on the web seems to say the chips actually went
down to 1 MB or less.
I think before doing this change, it would be good to understand where
the weird 9MB in libvirt/virt-manager came from, and what the limits of
the emulator/drivers are.
Also, is there something broken at the moment - why make the change?
Dave
> Signed-off-by: Gonglei
> ---
> For isa cirrus vga device, its' init function has been droped at
> commit db895a1e6a97e919f9b86d60c969377357b05066. I have no idea how adding
> check on isa_cirrus_vga device. Any ideas? Thanks.
>
> hw/display/cirrus_vga.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
> index d1afc76..5fec068 100644
> --- a/hw/display/cirrus_vga.c
> +++ b/hw/display/cirrus_vga.c
> @@ -2959,6 +2959,13 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev)
> PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
> int16_t device_id = pc->device_id;
>
> + /* follow real hardware, cirrus card emulated has 4 MB video memory.
> + Also accept 8 MB/16 MB for backward compatibility. */
> + if (s->vga.vram_size_mb != 4 || s->vga.vram_size_mb != 8 ||
> + s->vga.vram_size_mb != 16) {
> + error_report("Invalid cirrus_vga ram size '%u'\n",
> s->vga.vram_size_mb);
> + return -1;
> + }
> /* setup VGA */
> vga_common_init(&s->vga, OBJECT(dev), true);
> cirrus_init_common(s, OBJECT(dev), device_id, 1, pci_address_space(dev),
> --
> 1.7.12.4
>
>
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PULL 0/9] Tracing patches
On 7 May 2014 18:16, Stefan Hajnoczi wrote: > I forgot to open the QEMU 2.1 tracing queue. Let's get started! > > The following changes since commit 9898370497da3f18e0c9555b65c858eabc78ab50: > > Merge remote-tracking branch 'remotes/kraxel/tags/pull-smbios-2' into > staging (2014-05-06 12:23:05 +0100) > > are available in the git repository at: > > > git://github.com/stefanha/qemu.git tags/tracing-pull-request > > for you to fetch changes up to e00e36fb913217d49f57cc19d8d605270dd82bc5: > > configure: Show trace output file conditionally (2014-05-07 19:07:18 +0200) Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH] gluster: Correctly propagate errors when volume isn't accessible
On Fri, May 09, 2014 at 12:08:10PM +0200, Peter Krempa wrote:
> The docs for glfs_init suggest that the function sets errno on every
> failure. In fact it doesn't. As other functions such as
> qemu_gluster_open() in the gluster block code report their errors based
> on this fact we need to make sure that errno is set on each failure.
>
> This fixes a crash of qemu-img/qemu when a gluster brick isn't
> accessible from given host while the server serving the volume
> description is.
>
> Thread 1 (Thread 0x77fba740 (LWP 203880)):
> #0 0x777673f8 in glfs_lseek () from /usr/lib64/libgfapi.so.0
> #1 0x55574a68 in qemu_gluster_getlength ()
> #2 0x55565742 in refresh_total_sectors ()
> #3 0x5556914f in bdrv_open_common ()
> #4 0x5556e8e8 in bdrv_open ()
> #5 0x5556f02f in bdrv_open_image ()
> #6 0x5556e5f6 in bdrv_open ()
> #7 0x555c5775 in bdrv_new_open ()
> #8 0x555c5b91 in img_info ()
> #9 0x762c9c05 in __libc_start_main () from /lib64/libc.so.6
> #10 0x555648ad in _start ()
> ---
>
> Notes:
> I'm also going to report a bug in libgfapi too.
>
> block/gluster.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
Please use scripts/checkpatch.pl to check coding style in the future. I
added {} around the if statement body. QEMU always uses curlies even
for 1-statement bodies.
Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block
Stefan
Re: [Qemu-devel] [PATCH v2] cirrus_vga: adding sanity check for vram size
Am 09.05.2014 13:04, schrieb arei.gong...@huawei.com:
> From: Gonglei
>
> when configure a invalid vram size for cirrus card, such as less
> 2 MB, which will crash qemu. Follow the real hardware, the cirrus
> card has 4 MB video memory. Also for backward compatibility, accept
> 8 MB and 16 MB vram size.
>
> Signed-off-by: Gonglei
> ---
> v2: fix checkpatch fails.
> "WARNING: suspect code indent for conditional statements (5, 9)"
> maybe not a real warning.
>
> hw/display/cirrus_vga.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
> index d1afc76..399a2ef 100644
> --- a/hw/display/cirrus_vga.c
> +++ b/hw/display/cirrus_vga.c
> @@ -2959,6 +2959,14 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev)
> PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
> int16_t device_id = pc->device_id;
>
> + /* follow real hardware, cirrus card emulated has 4 MB video memory.
> + Also accept 8 MB/16 MB for backward compatibility. */
> + if (s->vga.vram_size_mb != 4 || s->vga.vram_size_mb != 8 ||
> + s->vga.vram_size_mb != 16) {
Apart from the logic bug mjt already pointed out, I note that this check
is in the PCI initfn. Should the same restriction also apply for the ISA
version of the device?
> + error_report("Invalid cirrus_vga ram size '%u'\n",
> + s->vga.vram_size_mb);
Thanks for using our new error_report(). It does not require a trailing
\n though.
Regards,
Andreas
> + return -1;
> + }
> /* setup VGA */
> vga_common_init(&s->vga, OBJECT(dev), true);
> cirrus_init_common(s, OBJECT(dev), device_id, 1, pci_address_space(dev),
--
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] QEMU 2.1 release schedule?
On 9 May 2014 02:07, Michael Roth wrote: > Quoting Peter Maydell (2014-05-08 08:48:01) >> Paolo suggested on IRC: >> >> > soft freeze mid june, hard freeze beginning of july, >> > release end of july or beginning of august? >> >> Which works for me. Some concrete dates: >> * 17 June: softfreeze and rc0 >>[ 2 week softfreeze period ] >> * 1 July: hardfreeze >>[ 4 week hardfreeze period, aiming for >> rc1, rc2, rc3 at about weekly intervals ] >> * 29 July: release >> >> Michael: does that schedule work for you, given you'd >> be doing the tarball-rolling parts? > > Looks good, don't see any issues with the proposed schedule on my end. Cool. I've put those dates up on http://wiki.qemu.org/Planning/2.1 and created a skeleton changelog on http://wiki.qemu.org/ChangeLog/2.1 PS: could somebody with write access to http://wiki.qemu.org/Download fix the 2.0 changelog link to point at ChangeLog/2.0 rather than ChangeLog/Next? The latter is now redirecting to the 2.1 page (as it should). thanks -- PMM
Re: [Qemu-devel] [PATCH] cirrus_vga: adding sanity check for vram size
Am 09.05.2014 12:59, schrieb Gonglei (Arei): >> -Original Message- >> From: Gerd Hoffmann [mailto:kra...@redhat.com] >> Sent: Friday, May 09, 2014 6:55 PM >> To: Gonglei (Arei) >> Cc: qemu-devel@nongnu.org; afaer...@suse.de; m...@redhat.com; >> pbonz...@redhat.com; Huangweidong (C); Blue Swirl >> Subject: Re: [PATCH] cirrus_vga: adding sanity check for vram size >> [checkpatch >> false positive?] >> >> Hi, >> >>> BTW, what's your opinion about isa cirrus vga device, Gerd? >> >> I'd do the same check there. >> > But isa_cirrus_vga_realizefn() has no return value for judgment. > Adding the isa init function back? No, realizefn is the replacement for the old initfns. Adding qtests for verifying that things still work is what's holding up the conversion of PCI devices. You need to set *errp via error_setg(errp, "..."); instead of error_report() and then just return. 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] kvmclock: Ensure time in migration never goes backward
Il 09/05/2014 04:28, Marcelo Tosatti ha scritto: Alex, Unability to upgrade systems is not an excuse to fix the bug in the wrong place. It may be an excuse to fix the bug in both places though. Paolo
Re: [Qemu-devel] [PATCH] cirrus_vga: adding sanity check for vram size
Hi, > virt-manager/libvirt seems to default to 9 MByte of Vram for cirrus, > so this would break a lot of setups. It wouldn't. libvirt sticks that into the xml, but it doesn't set any qemu parameters. The libvirt parameter actually predates the qemu property for setting the size. > Looking at datasheets on the web seems to say the chips actually went > down to 1 MB or less. I have my doubts we emulate that correctly (register telling the guest how much memory is actually there etc.). Also it is pretty much useless these days, even the 4MB imply serious constrains when FullHD displays are commonplace. Newer cirrus drivers such as the kernel's drm driver are specifically written to qemu's cirrus cards, I have my doubs that they are prepared to handle 1MB cirrus cards correctly. Bottom line: Allowing less than 4MB is asking for trouble for no good reason ;) cheers, Gerd
Re: [Qemu-devel] [PULL 00/38] QMP queue
On 8 May 2014 19:52, Luiz Capitulino wrote: > The following changes since commit 6b342cc9c872e82620fdd32730cd92affa8a19b3: > > Merge remote-tracking branch 'remotes/spice/tags/pull-spice-7' into staging > (2014-05-08 10:57:25 +0100) > > are available in the git repository at: > > > git://repo.or.cz/qemu/qmp-unstable.git queue/qmp > > for you to fetch changes up to 4c56cbae6908cc597842e86e523886bb8ea8cfb8: > > Revert "qapi: Clean up superfluous null check in qapi_dealloc_type_str()" > (2014-05-08 14:20:26 -0400) Hi; this doesn't build for w32, I'm afraid: /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c: In function ‘qga_vss_fsfreeze’: /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c:160: error: ‘err’ undeclared (first use in this function) /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c:160: error: (Each undeclared identifier is reported only once /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c:160: error: for each function it appears in.) thanks -- PMM
Re: [Qemu-devel] [PATCH] cirrus_vga: adding sanity check for vram size
* Gerd Hoffmann (kra...@redhat.com) wrote: > Hi, > > > virt-manager/libvirt seems to default to 9 MByte of Vram for cirrus, > > so this would break a lot of setups. > > It wouldn't. libvirt sticks that into the xml, but it doesn't set any > qemu parameters. The libvirt parameter actually predates the qemu > property for setting the size. Yeuch messy. > > Looking at datasheets on the web seems to say the chips actually went > > down to 1 MB or less. > > I have my doubts we emulate that correctly (register telling the guest > how much memory is actually there etc.). Also it is pretty much useless > these days, even the 4MB imply serious constrains when FullHD displays > are commonplace. Newer cirrus drivers such as the kernel's drm driver > are specifically written to qemu's cirrus cards, I have my doubs that > they are prepared to handle 1MB cirrus cards correctly. > > Bottom line: Allowing less than 4MB is asking for trouble for no good > reason ;) OK, so checking for 4MB/8MB/16MB is probably safe, and it also would have the benefit of shouting if someone fixed libvirt and it started trying to configure a 9MB version. Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH] cirrus_vga: adding sanity check for vram size
Il 09/05/2014 13:18, Dr. David Alan Gilbert ha scritto: * arei.gong...@huawei.com (arei.gong...@huawei.com) wrote: From: Gonglei when configure a invalid vram size for cirrus card, such as less 2 MB, which will crash qemu. Follow the real hardware, the cirrus card has 4 MB video memory. Also for backward compatibility, accept 8 MB and 16 MB vram size. virt-manager/libvirt seems to default to 9 MByte of Vram for cirrus, so this would break a lot of setups. libvirt is broken and doesn't use the vram property for anything but QXL. Paolo
Re: [Qemu-devel] [PATCH 1/6] xics: add flags for interrupts
On Thu, May 8, 2014 at 11:12 PM, Alexey Kardashevskiy wrote: >> There are >> a couple ways to mitigate this type of situation by using alternative >> data structures to inform the loop traversal. I don't know if it is >> worth the effort, though. > > Here I lost you :) If I read the code correctly, the problem I'm wondering about is that the loop will waste time traversing the array when there are only unallocated interrupts from the current index to the end. For example, if the interrupt array is 256 entries and the highest index that is allocated is 16, the outside loop will traverse all 256 entries while it should have exited after the 16th. To mitigate this you could keep a shadow index of the current highest allocated index and check for that in the outside loop. Or you could maintain a shadow linked list that only includes allocated array entries and just traverse that list. Each element on the list would be an allocated entry in the interrupt array. > btw I just realized that in patch#2 it should be xics_find_source instead > of xics_find_server. There are many interrupt servers already and just one > interrupt source (we could have many like one per PHB or something like > that but we are not there yet), this is what I meant.
Re: [Qemu-devel] [PATCH 0/5] Clean up around bdrv_getlength()
On Fri, May 09, 2014 at 11:48:13AM +0200, Markus Armbruster wrote: > Issues addressed in this series: > > * BlockDriver method bdrv_getlength() generally returns -errno, but > some implementations return -1 instead. Fix them [PATCH 1]. > > * Frequent conversions between sectors and bytes complicate the code > needlessly. Clean up some [PATCH 2+3]. > > * bdrv_getlength() always returns a multiple of BDRV_SECTOR_SIZE, but > some places appear to be confused about that, and align the result > up or down. Don't [PATCH 4]. > > * bdrv_get_geometry() hides errors. Don't use it in places where > errors should be detected [PATCH 5]. > > Issues not addressed: > > * There are quite a few literals left in the code where > BDRV_SECTOR_SIZE, BDRV_SECTOR_BITS or BDRV_SECTOR_MASK should be > used instead. > > * Error handling is missing in places, but it's not always obvious > whether errors can actually happen, and if yes, how to handle them. > > Markus Armbruster (5): > raw-posix: Fix raw_getlength() to always return -errno on error > block: New bdrv_nb_sectors() > block: Use bdrv_nb_sectors() when sectors, not bytes are wanted > block: Drop superfluous aligning of bdrv_getlength()'s value > block: Avoid bdrv_get_geometry() where errors should be detected > > block-migration.c | 9 +++-- > block.c | 81 > block/qapi.c | 14 +--- > block/qcow2.c | 3 +- > block/raw-posix.c | 28 > block/vmdk.c | 5 ++- > include/block/block.h | 1 + > qemu-img.c| 93 > ++- > 8 files changed, 147 insertions(+), 87 deletions(-) > > -- > 1.8.1.4 > > Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
Re: [Qemu-devel] [PATCH v3 1/4] vl.c: extend -m option to support options for memory hotplug
On 07/05/14 20:05, Matthew Rosato wrote:
> From: Igor Mammedov
>
> From: Igor Mammedov
>
> Add following parameters:
> "slots" - total number of hotplug memory slots
> "maxmem" - maximum possible memory
>
> "slots" and "maxmem" should go in pair and "maxmem" should be greater
> than "mem" for memory hotplug to be enabled.
>
> Signed-off-by: Matthew Rosato
I would prefer if Igor could resend his latest version and one of the overall
maintainer applies this as it is not s390-specific.
What happened to the x86 implementation?
Christian
> ---
> include/hw/boards.h |2 ++
> qemu-options.hx |9 ++---
> vl.c| 51
> +++
> 3 files changed, 59 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 4345bd0..5b3a903 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -11,6 +11,8 @@
> typedef struct QEMUMachineInitArgs {
> const MachineClass *machine;
> ram_addr_t ram_size;
> +ram_addr_t maxram_size;
> +uint64_t ram_slots;
> const char *boot_order;
> const char *kernel_filename;
> const char *kernel_cmdline;
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 781af14..c6411c4 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -210,17 +210,20 @@ use is discouraged as it may be removed from future
> versions.
> ETEXI
>
> DEF("m", HAS_ARG, QEMU_OPTION_m,
> -"-m [size=]megs\n"
> +"-m[emory] [size=]megs[,slots=n,maxmem=size]\n"
> "configure guest RAM\n"
> "size: initial amount of guest memory (default: "
> -stringify(DEFAULT_RAM_SIZE) "MiB)\n",
> +stringify(DEFAULT_RAM_SIZE) "MiB)\n"
> +"slots: number of hotplug slots (default: none)\n"
> +"maxmem: maximum amount of guest memory (default:
> none)\n",
> QEMU_ARCH_ALL)
> STEXI
> @item -m [size=]@var{megs}
> @findex -m
> Set virtual RAM size to @var{megs} megabytes. Default is 128 MiB.
> Optionally,
> a suffix of ``M'' or ``G'' can be used to signify a value in megabytes or
> -gigabytes respectively.
> +gigabytes respectively. Optional pair @var{slots}, @var{maxmem} could be used
> +to set amount of hotluggable memory slots and possible maximum amount of
> memory.
> ETEXI
>
> DEF("mem-path", HAS_ARG, QEMU_OPTION_mempath,
> diff --git a/vl.c b/vl.c
> index 73e0661..36ad82c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -520,6 +520,14 @@ static QemuOptsList qemu_mem_opts = {
> .name = "size",
> .type = QEMU_OPT_SIZE,
> },
> +{
> +.name = "slots",
> +.type = QEMU_OPT_NUMBER,
> +},
> +{
> +.name = "maxmem",
> +.type = QEMU_OPT_SIZE,
> +},
> { /* end of list */ }
> },
> };
> @@ -2988,6 +2996,8 @@ int main(int argc, char **argv, char **envp)
> const char *trace_file = NULL;
> const ram_addr_t default_ram_size = (ram_addr_t)DEFAULT_RAM_SIZE *
> 1024 * 1024;
> +ram_addr_t maxram_size = default_ram_size;
> +uint64_t ram_slots = 0;
>
> atexit(qemu_run_exit_notifiers);
> error_set_progname(argv[0]);
> @@ -3324,6 +3334,7 @@ int main(int argc, char **argv, char **envp)
> case QEMU_OPTION_m: {
> uint64_t sz;
> const char *mem_str;
> +const char *maxmem_str, *slots_str;
>
> opts = qemu_opts_parse(qemu_find_opts("memory"),
> optarg, 1);
> @@ -3365,6 +3376,44 @@ int main(int argc, char **argv, char **envp)
> error_report("ram size too large");
> exit(EXIT_FAILURE);
> }
> +
> +maxmem_str = qemu_opt_get(opts, "maxmem");
> +slots_str = qemu_opt_get(opts, "slots");
> +if (maxmem_str && slots_str) {
> +uint64_t slots;
> +
> +sz = qemu_opt_get_size(opts, "maxmem", 0);
> +if (sz < ram_size) {
> +fprintf(stderr, "qemu: invalid -m option value:
> maxmem "
> +"(%" PRIu64 ") <= initial memory (%"
> +PRIu64 ")\n", sz, ram_size);
> +exit(EXIT_FAILURE);
> +}
> +
> +slots = qemu_opt_get_number(opts, "slots", 0);
> +if ((sz > ram_size) && !slots) {
> +fprintf(stderr, "qemu: invalid -m option value:
> maxmem "
> +"(%" PRIu64 ") more than initial memory (%"
> +PRIu64 ") but no hotplug slots where "
> +"specified\n", sz, ram_size);
> +exit(EXIT_FAILURE);
> +}
> +
> +if ((
Re: [Qemu-devel] Question about RAM migration flags
> Am 09.05.2014 um 11:43 schrieb "Dr. David Alan Gilbert" : > > * Peter Lieven (p...@kamp.de) wrote: >> Hi, >> >> while working on ram migration and reading through the code I realized that >> qemu does not stop loading a saved VM or rejecting an incoming migration >> if there is a flag in the stream that it does not understand. An unknown flag >> is simply ignored. >> >> In the block migration code there is a catch at the end complaining about >> unknown flags, but in RAM migration there isn't. >> >> Is this on purpose or an error? > > I think it's in error; the code doesn't have much checking. i will prepare a patch. Peter > > Dave > -- > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH] gluster: Correctly propagate errors when volume isn't accessible
On 05/09/14 13:39, Stefan Hajnoczi wrote:
> On Fri, May 09, 2014 at 12:08:10PM +0200, Peter Krempa wrote:
...
>
> Please use scripts/checkpatch.pl to check coding style in the future. I
> added {} around the if statement body. QEMU always uses curlies even
> for 1-statement bodies.
Ah, right, sorry about that. I'm used to libvirt's coding style which
allows it. I'll use the patch checker next time.
>
> Thanks, applied to my block tree:
> https://github.com/stefanha/qemu/commits/block
Thanks.
>
> Stefan
>
Peter
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PULL 00/38] QMP queue
Peter Maydell writes:
> On 8 May 2014 19:52, Luiz Capitulino wrote:
>> The following changes since commit 6b342cc9c872e82620fdd32730cd92affa8a19b3:
>>
>> Merge remote-tracking branch 'remotes/spice/tags/pull-spice-7'
>> into staging (2014-05-08 10:57:25 +0100)
>>
>> are available in the git repository at:
>>
>>
>> git://repo.or.cz/qemu/qmp-unstable.git queue/qmp
>>
>> for you to fetch changes up to 4c56cbae6908cc597842e86e523886bb8ea8cfb8:
>>
>> Revert "qapi: Clean up superfluous null check in
>> qapi_dealloc_type_str()" (2014-05-08 14:20:26 -0400)
>
> Hi; this doesn't build for w32, I'm afraid:
>
> /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c: In function
> ‘qga_vss_fsfreeze’:
> /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c:160: error:
> ‘err’ undeclared (first use in this function)
> /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c:160: error:
> (Each undeclared identifier is reported only once
> /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c:160: error: for
> each function it appears in.)
My fault! Apologies...
The obvious fix needs to be squashed into "qga: Consistently name Error
** objects errp, and not err":
diff --git a/qga/vss-win32.c b/qga/vss-win32.c
index b17c909..0e40957 100644
--- a/qga/vss-win32.c
+++ b/qga/vss-win32.c
@@ -157,7 +157,7 @@ void qga_vss_fsfreeze(int *nr_volume, Error **errp, bool
freeze)
func = (QGAVSSRequesterFunc)GetProcAddress(provider_lib, func_name);
if (!func) {
-error_setg_win32(err, GetLastError(), "failed to load %s from %s",
+error_setg_win32(errp, GetLastError(), "failed to load %s from %s",
func_name, QGA_VSS_DLL);
return;
}
Re: [Qemu-devel] [PATCH] hw/arm/spitz: Avoid clash with Windows header symbol MOD_SHIFT
On 20 February 2014 19:45, Peter Maydell wrote: > The Windows headers provided by MinGW define MOD_SHIFT. Avoid > it by using SPITZ_MOD_* for our constants here. > > Signed-off-by: Peter Maydell > --- > The other approach would be just to #undef MOD_SHIFT, (and > looking back through the archives I see Stefan posted a patch > to do just that last year) but I think it's cleaner to do this. > > I replaced a few of the /* apostrophe */ keysym names with > the symbols just to keep us under the 80 column limit... > --- > hw/arm/spitz.c | 108 > +++-- > 1 file changed, 58 insertions(+), 50 deletions(-) I see the patchset to try to reduce the scope where windows.h is pulled in got a bit bogged down in review. Stefan, are you still planning to move forward with that, or should we maybe just apply this patch fror now? I think this is the only w32 build warning currently... thanks -- PMM
Re: [Qemu-devel] [PULL 00/38] QMP queue
On Fri, 09 May 2014 14:54:45 +0200 Markus Armbruster wrote: > Peter Maydell writes: > > > On 8 May 2014 19:52, Luiz Capitulino wrote: > >> The following changes since commit > >> 6b342cc9c872e82620fdd32730cd92affa8a19b3: > >> > >> Merge remote-tracking branch 'remotes/spice/tags/pull-spice-7' > >> into staging (2014-05-08 10:57:25 +0100) > >> > >> are available in the git repository at: > >> > >> > >> git://repo.or.cz/qemu/qmp-unstable.git queue/qmp > >> > >> for you to fetch changes up to 4c56cbae6908cc597842e86e523886bb8ea8cfb8: > >> > >> Revert "qapi: Clean up superfluous null check in > >> qapi_dealloc_type_str()" (2014-05-08 14:20:26 -0400) > > > > Hi; this doesn't build for w32, I'm afraid: > > > > /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c: In function > > ‘qga_vss_fsfreeze’: > > /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c:160: error: > > ‘err’ undeclared (first use in this function) > > /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c:160: error: > > (Each undeclared identifier is reported only once > > /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c:160: error: for > > each function it appears in.) > > My fault! Apologies... > > The obvious fix needs to be squashed into "qga: Consistently name Error > ** objects errp, and not err": Yep. I'm testing this right now. A clear indication that neither you or me are testing builds on w32/64. We need a buildbot badly.
Re: [Qemu-devel] [PATCH v18 00/15] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD
No longer applies. It's been four weeks :( Fam Zheng writes: > v18: Address reviewing comments from Jeff and Eric. Rebased to current master. > Side by side diff from v17: http://bit.ly/1oO2Fvt > > [01/15] block: Add BlockOpType enum > Add Jeff's reviewed-by. > > [02/15] block: Introduce op_blockers to BlockDriverState > Add Jeff's reviewed-by. > > [03/15] block: Replace in_use with operation blocker > Add Jeff's reviewed-by. > > [04/15] block: Move op_blocker check from block_job_create to its caller > Add Jeff's reviewed-by. > > [05/15] block: Add bdrv_set_backing_hd() > Don't unset bs->backing_file and bs->backing_format when > backing_hd==NULL, because qcow2_close() will save these into image > header. > > [08/15] block: Support dropping active in bdrv_drop_intermediate > Swap parameters for bdrv_swap: > bdrv_swap(active, base); -> bdrv_swap(base, active); > Use bdrv_set_backing_hd(). > > [10/15] commit: Use bdrv_drop_intermediate > New. (Jeff) > > [11/15] qmp: Add command 'blockdev-backup' > Since 2.0 -> Since 2.1. (Eric) > > [13/15] block: Add blockdev-backup to transaction > Comment "Since 2.1" for blockdev-backup. (Eric) > > [15/15] qemu-iotests: Image fleecing test case 089 > Case number 083 -> 089.
[Qemu-devel] Migration from older Qemu to Qemu 2.0.0 does not work
Hello list, i was trying to migrate older Qemu (1.5 and 1.7.2) to a machine running Qemu 2.0. I started the target machine with: -machine type=pc-i440fx-1.5 / -machine type=pc-i440fx-1.7 But the migration simply fails. Migrating Qemu 2.0 to Qemu 2.0 succeeds. I see no output at the monitor of Qemu 2.0. # migrate -d tcp:a.b.c.d:6000 # info migrate capabilities: xbzrle: off x-rdma-pin-all: off auto-converge: on zero-blocks: off Migration status: failed total time: 0 milliseconds The target machine is still running at this point with no output. Stefan
[Qemu-devel] [PATCH] hw/audio/intel-hda: Avoid shift into sign bit
Add a U suffix to avoid shifting into the sign bit (which is
undefined behaviour in C).
Signed-off-by: Peter Maydell
---
Another one from clang's sanitizer...
hw/audio/intel-hda.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index d41f82c..0cec2d2 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -245,7 +245,7 @@ static void intel_hda_update_int_sts(IntelHDAState *d)
/* update global status */
if (sts & d->int_ctl) {
-sts |= (1 << 31);
+sts |= (1U << 31);
}
d->int_sts = sts;
@@ -257,7 +257,7 @@ static void intel_hda_update_irq(IntelHDAState *d)
int level;
intel_hda_update_int_sts(d);
-if (d->int_sts & (1 << 31) && d->int_ctl & (1 << 31)) {
+if (d->int_sts & (1U << 31) && d->int_ctl & (1U << 31)) {
level = 1;
} else {
level = 0;
--
1.9.2
Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
On Fri, May 09, 2014 at 11:26:35AM +0100, Ian Campbell wrote: > On Fri, 2014-05-09 at 10:15 +, Gonglei (Arei) wrote: > > Hi, > > > > First, please forgive me for my bad English. > > It's so sad. > > > > > -Original Message- > > > From: Ian Campbell [mailto:ian.campb...@citrix.com] > > > Sent: Friday, May 09, 2014 5:57 PM > > > To: Gonglei (Arei) > > > Cc: Jan Beulich; xen-de...@lists.xen.org; anthony.per...@citrix.com; > > > stefano.stabell...@eu.citrix.com; johannes.kra...@googlemail.com; Gaowei > > > (UVP); Hanweidong (Randy); Huangweidong (C); ke...@koconnor.net; > > > fabio.fant...@m2r.biz; qemu-devel@nongnu.org; m...@redhat.com > > > Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods > > > for PCIslots that support hotplug by runtime patching > > > > > > On Fri, 2014-05-09 at 09:45 +, Gonglei (Arei) wrote: > > > > > And it also seem pretty pointless to send a v4 without addressing > > > > > all comments you got on v3. > > > > > > > > > I don't think so. I have absorbed Ian's all suggestion on v3. And for > > > > other > > > > questions have been answered too, in despite of is me or not. > > > > > > Actually you haven't answered "Why is runtime patching the only > > > option here?" which was originally phrased as: > > > > > Which appears to involve an awful lot of jumping through hoops... > > > > > Please > > > > > can you explain why it is necessary, as opposed to e.g. using a > > > > > dynamic > > > > > set of SSDTs? > > > > > Ian, I understand your mean now, which consider our method to address > > this issue is maybe unnecessary, right? And you suggest us to use a dynamic > > set of SSDTs. > > Really what I'm asking is what set of constraints and requirements led > you to this particular solution. > > I think the method seems complicated, and I'd therefore like to know why > it was preferred over other alternatives, or perhaps why it is the only > option. > > > TBH I don't know more about the dynamic SSDTs, if you have any details, > > tell me please, thanks in advance! > > I'm not an ACPI expert, but AIUI an SSDT is essentially a little piece > of DSDT which is grafted onto the main DSDT at runtime by the OSPM. They > make it somewhat easier for BIOS (or ACPI table) authors to include or > exclude functionality at runtime, perhaps on a physical system in > response to a user changing something in the BIOS setup screens. In Xen > we appear to use SSDTs for HPET, TPM and S3/S4 functionality, depending > on the guest configuration > (hvmloader/acpi/build.c:construct_secondary_tables()). Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk of the ACPI PCI stuff can be moved there ? How would this work with the 'secondary emulator' patches that do all of this PCI hotplug in the hypervisor? (CC-ing the author of said patches). > > Ian. > > > ___ > Xen-devel mailing list > xen-de...@lists.xen.org > http://lists.xen.org/xen-devel
Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
On Fri, 2014-05-09 at 09:25 -0400, Konrad Rzeszutek Wilk wrote: > On Fri, May 09, 2014 at 11:26:35AM +0100, Ian Campbell wrote: > > On Fri, 2014-05-09 at 10:15 +, Gonglei (Arei) wrote: > > > Hi, > > > > > > First, please forgive me for my bad English. > > > It's so sad. > > > > > > > -Original Message- > > > > From: Ian Campbell [mailto:ian.campb...@citrix.com] > > > > Sent: Friday, May 09, 2014 5:57 PM > > > > To: Gonglei (Arei) > > > > Cc: Jan Beulich; xen-de...@lists.xen.org; anthony.per...@citrix.com; > > > > stefano.stabell...@eu.citrix.com; johannes.kra...@googlemail.com; Gaowei > > > > (UVP); Hanweidong (Randy); Huangweidong (C); ke...@koconnor.net; > > > > fabio.fant...@m2r.biz; qemu-devel@nongnu.org; m...@redhat.com > > > > Subject: Re: [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 > > > > methods > > > > for PCIslots that support hotplug by runtime patching > > > > > > > > On Fri, 2014-05-09 at 09:45 +, Gonglei (Arei) wrote: > > > > > > And it also seem pretty pointless to send a v4 without addressing > > > > > > all comments you got on v3. > > > > > > > > > > > I don't think so. I have absorbed Ian's all suggestion on v3. And for > > > > > other > > > > > questions have been answered too, in despite of is me or not. > > > > > > > > Actually you haven't answered "Why is runtime patching the only > > > > option here?" which was originally phrased as: > > > > > > Which appears to involve an awful lot of jumping through hoops... > > > > > > Please > > > > > > can you explain why it is necessary, as opposed to e.g. using a > > > > > > dynamic > > > > > > set of SSDTs? > > > > > > > Ian, I understand your mean now, which consider our method to address > > > this issue is maybe unnecessary, right? And you suggest us to use a > > > dynamic > > > set of SSDTs. > > > > Really what I'm asking is what set of constraints and requirements led > > you to this particular solution. > > > > I think the method seems complicated, and I'd therefore like to know why > > it was preferred over other alternatives, or perhaps why it is the only > > option. > > > > > TBH I don't know more about the dynamic SSDTs, if you have any details, > > > tell me please, thanks in advance! > > > > I'm not an ACPI expert, but AIUI an SSDT is essentially a little piece > > of DSDT which is grafted onto the main DSDT at runtime by the OSPM. They > > make it somewhat easier for BIOS (or ACPI table) authors to include or > > exclude functionality at runtime, perhaps on a physical system in > > response to a user changing something in the BIOS setup screens. In Xen > > we appear to use SSDTs for HPET, TPM and S3/S4 functionality, depending > > on the guest configuration > > (hvmloader/acpi/build.c:construct_secondary_tables()). > > Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk > of the ACPI PCI stuff can be moved there ? I think it can "shadow" or extend existing DSDT stuff, I don't think it can patch as sych. But here we want to dynamically add an entire method I think? (or hide, but I don't think that is possible). > How would this work with the 'secondary emulator' patches that > do all of this PCI hotplug in the hypervisor? It should just mean that the magic I/O port protocol becomes backed by Xen, although now you mentioned it I suppose it will be necessary for Xen to know the answer now ;-) > (CC-ing the author > of said patches). I thought I did earlier, perhaps I forgot or changed my mind. Ian.
Re: [Qemu-devel] [PATCH v3 12/16] tcg-s390: Define TCG_TARGET_INSN_UNIT_SIZE
On 28 April 2014 20:28, Richard Henderson wrote:
> And use tcg pointer differencing functions as appropriate.
>
> Signed-off-by: Richard Henderson
> ---
> tcg/s390/tcg-target.c | 91
> ---
> tcg/s390/tcg-target.h | 2 ++
> 2 files changed, 45 insertions(+), 48 deletions(-)
>
> diff --git a/tcg/s390/tcg-target.c b/tcg/s390/tcg-target.c
> index 1d912a7..ae1be1c 100644
> --- a/tcg/s390/tcg-target.c
> +++ b/tcg/s390/tcg-target.c
> @@ -320,7 +320,7 @@ static const uint8_t tcg_cond_to_ltr_cond[] = {
> #ifdef CONFIG_SOFTMMU
> /* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr,
> int mmu_idx) */
> -static const void * const qemu_ld_helpers[4] = {
> +static void * const qemu_ld_helpers[4] = {
> helper_ldb_mmu,
> helper_ldw_mmu,
> helper_ldl_mmu,
> @@ -329,7 +329,7 @@ static const void * const qemu_ld_helpers[4] = {
>
> /* helper signature: helper_st_mmu(CPUState *env, target_ulong addr,
> uintxx_t val, int mmu_idx) */
> -static const void * const qemu_st_helpers[4] = {
> +static void * const qemu_st_helpers[4] = {
> helper_stb_mmu,
> helper_stw_mmu,
> helper_stl_mmu,
Why do these lose the 'const' ?
Patch looks ok otherwise from a quick scan.
thanks
-- PMM
Re: [Qemu-devel] [PATCH v18 00/15] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD
On Fri, 05/09 15:08, Markus Armbruster wrote: > No longer applies. It's been four weeks :( Hi Markus, Thanks for noticing this, I'll rebase and send another revision now. Fam > > Fam Zheng writes: > > > v18: Address reviewing comments from Jeff and Eric. Rebased to current > > master. > > Side by side diff from v17: http://bit.ly/1oO2Fvt > > > > [01/15] block: Add BlockOpType enum > > Add Jeff's reviewed-by. > > > > [02/15] block: Introduce op_blockers to BlockDriverState > > Add Jeff's reviewed-by. > > > > [03/15] block: Replace in_use with operation blocker > > Add Jeff's reviewed-by. > > > > [04/15] block: Move op_blocker check from block_job_create to its caller > > Add Jeff's reviewed-by. > > > > [05/15] block: Add bdrv_set_backing_hd() > > Don't unset bs->backing_file and bs->backing_format when > > backing_hd==NULL, because qcow2_close() will save these into > > image > > header. > > > > [08/15] block: Support dropping active in bdrv_drop_intermediate > > Swap parameters for bdrv_swap: > > bdrv_swap(active, base); -> bdrv_swap(base, active); > > Use bdrv_set_backing_hd(). > > > > [10/15] commit: Use bdrv_drop_intermediate > > New. (Jeff) > > > > [11/15] qmp: Add command 'blockdev-backup' > > Since 2.0 -> Since 2.1. (Eric) > > > > [13/15] block: Add blockdev-backup to transaction > > Comment "Since 2.1" for blockdev-backup. (Eric) > > > > [15/15] qemu-iotests: Image fleecing test case 089 > > Case number 083 -> 089. >
Re: [Qemu-devel] [PATCH v3 14/16] tcg-mips: Define TCG_TARGET_INSN_UNIT_SIZE
On 28 April 2014 20:28, Richard Henderson wrote: > And use tcg pointer differencing functions as appropriate. > > Cc: Aurelien Jarno > Signed-off-by: Richard Henderson Reviewed-by: Peter Maydell -- PMM
Re: [Qemu-devel] Migration from older Qemu to Qemu 2.0.0 does not work
* Stefan Priebe - Profihost AG (s.pri...@profihost.ag) wrote: > Hello list, > > i was trying to migrate older Qemu (1.5 and 1.7.2) to a machine running > Qemu 2.0. > > I started the target machine with: > > -machine type=pc-i440fx-1.5 / -machine type=pc-i440fx-1.7 I'd expect you to have to run with the same machine type on both sides. I ran some simple virt-test migrate tests (just the basic one) and got v1.5.3->v1.6.2 v1.5.3->v1.7.1 v1.5.3->v2.0.0-rc1 working for most machine types, pc-i440fx-1.5 passed with all of those. Note that's only the simplest test. Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v3 15/16] tci: Define TCG_TARGET_INSN_UNIT_SIZE
On 28 April 2014 20:28, Richard Henderson wrote: > And use tcg pointer differencing functions as appropriate. > > Cc: Stefan Weil > Signed-off-by: Richard Henderson > --- > tcg/tci/tcg-target.c | 19 +-- > tcg/tci/tcg-target.h | 1 + > 2 files changed, 14 insertions(+), 6 deletions(-) Reviewed-by: Peter Maydell though I note there are a number of functions that use the pattern uint8_t *old_code_ptr = s->code_ptr; [...emit stuff...] old_code_ptr[1] = s->code_ptr - old_code_ptr; which could perhaps use tcg_insn_unit * rather than uint8_t * ? thanks -- PMM
Re: [Qemu-devel] [PATCH] hw/audio/intel-hda: Avoid shift into sign bit
On Fr, 2014-05-09 at 14:22 +0100, Peter Maydell wrote: > Add a U suffix to avoid shifting into the sign bit (which is > undefined behaviour in C). Added to audio patch queue. thanks, Gerd
Re: [Qemu-devel] [PATCH v3 10/16] tcg-arm: Define TCG_TARGET_INSN_UNIT_SIZE
On 2 May 2014 16:24, Richard Henderson wrote: > On 05/02/2014 08:19 AM, Peter Maydell wrote: >> This change also confused me but we're again relying on movi32 >> generating correct-but-inefficient code now, right? > > Yes. IMO if we want to do a constant pool, let's do a proper one, > not just in a few places. Fair enough. I don't care very much about pre-v7 hosts anyway... Reviewed-by: Peter Maydell -- PMM
Re: [Qemu-devel] [PATCH v3 11/16] tcg-aarch64: Define TCG_TARGET_INSN_UNIT_SIZE
On 28 April 2014 20:28, Richard Henderson wrote: > And use tcg pointer differencing functions as appropriate. > > Cc: Claudio Fontana > Signed-off-by: Richard Henderson Reviewed-by: Peter Maydell -- PMM
Re: [Qemu-devel] [SeaBIOS] seabios release?
Hi, > > ... so it's time kick freeze + release candidate I guess ;) > > > > Are there any pending patches for the next release which should better > > make it into the release candidate? > > I just sent a few fixes to the mailing list (xhci msleep, romlayout.o > rebuild, int 1589 fix). I think these should go into the final > release. Unless there are comments on the above patches I'll push > them on Monday. > > I think we can then also start the feature freeze on Monday. Sounds good (and the patches look sane to me too btw). So lets tag the release candidate next monday, after applying the patches. cheers, Gerd
Re: [Qemu-devel] [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
Il 08/05/2014 13:23, Gonglei (Arei) ha scritto: Hi, -Original Message- From: Fabio Fantoni [mailto:fabio.fant...@m2r.biz] Sent: Thursday, May 08, 2014 4:17 PM To: Gonglei (Arei); qemu-devel@nongnu.org; xen-de...@lists.xen.org Cc: ian.campb...@citrix.com; jbeul...@suse.com; stefano.stabell...@eu.citrix.com; anthony.per...@citrix.com; Huangweidong (C); Hanweidong (Randy); Gaowei (UVP); Michael S. Tsirkin Subject: Re: [PATCH v3] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching Il 08/05/2014 06:12, Gonglei (Arei) ha scritto: Hi, In Xen platform, after using upstream qemu, the all of pci devices will show hotplug in the windows guest. In this situation, the windows guest may occur blue screen when VM' user click the icon of VGA card for trying unplug VGA card. However, we don't hope VM's user can do such dangerous operation, and showing all pci devices inside the guest OS is unfriendly. This is done by runtime patching: - Rename _EJ0 methods for PCI slots in DSDT to EJ0_:note that this has the same checksum, but is ignored by OSPM. - At compile time, look for these methods in ASL source,find the matching AML, and store the offsets of these methods in a table named aml_ej0_data. - At run time, go over aml_ej0_data, check which slots not support hotplug and patch the ACPI table, replacing _EJ0 with EJ0_. Signed-off-by: Gaowei Signed-off-by: Gonglei Tested-by: Fabio Fantoni Thanks for this very useful patch that avoid that unaware users or as mistake make windows domUs unusable. Thanks. I tried to quickly look at the patch to understand how to add some optional components, for example on my case the pv drivers, the audio card and the spice guest tools (see attachment) but I don't understand how to do it. Can someone give me some advices about it please? Maybe you can hard code at libxl__build_device_model_args_new() in tools/libxl/libxl_dm.c Best regards, -Gonglei I believe I not understand what you mean. About adding these components I already do this: - gplpv: http://wiki.xen.org/wiki/Xen_Windows_GplPv http://www.ejbdigital.com.au/gplpv - spice: http://xenbits.xen.org/docs/unstable/man/xl.cfg.5.html#spice_graphics_suppo rt http://www.spice-space.org/download/binaries/spice-guest-tools/ - audio: add soundhw="hda" in domU's xl cfg file This patch disables hotplug capabilities from essential pci devices. No, if pci devices do not support hotplug, which class's no_hotplug property will be set to 1, then the pci device will not show in guest os, opposite will be shown, such as your pv drivers and Intel hda card. Please see intel_hda_class_init_common() and xen_platform_class_init() functions. As a comparison, you can see vga devices' class init function cirrus_vga_class_init() has below code: k->no_hotplug = 1; If I understand what you mean wrong, please forgive me. I saw in qemu code that intel_hda_class_init_common() and xen_platform_class_init() functions (and probably also virtio serial one) not have k->no_hotplug but in windows domUs show them in hotplug devices list, also with this your patch applied. (see part of screenshoot in attachment) I not understand how to remove also them from windows hotplug devices list. Thanks for any reply and sorry for my bad english. What I mean if there is a way to prevent some other components being part of the above list of hotplug devices. Thanks for any reply and sorry for my bad english. Best regards, -Gonglei
[Qemu-devel] [PATCH v3 ] trace: Multi-backend tracing
Adds support to compile QEMU with multiple tracing backends at the same time.
For example, you can compile QEMU with:
$ ./configure --enable-trace-backends=ftrace,dtrace
Where 'ftrace' can be handy for having an in-flight record of events, and
'dtrace' can be later used to extract more information from the system.
This patch allows having both available without recompiling QEMU.
Signed-off-by: Lluís Vilanova
---
.travis.yml |8 +++--
Makefile |4 +--
Makefile.target |4 +--
configure | 44 +-
docs/tracing.txt |4 +--
qemu-io.c |2 +
scripts/tracetool.py | 41 ++--
scripts/tracetool/__init__.py | 22 +++
scripts/tracetool/backend/__init__.py | 15 +-
trace/Makefile.objs | 22 +++
trace/control-internal.h |4 +--
trace/control.c | 49 -
trace/control.h | 27 +++---
trace/default.c | 40 ---
trace/ftrace.c| 25 +
trace/ftrace.h|5 +++
trace/simple.c| 19 +
trace/simple.h|1 +
trace/stderr.c| 30
vl.c |4 +--
20 files changed, 144 insertions(+), 226 deletions(-)
delete mode 100644 trace/default.c
delete mode 100644 trace/stderr.c
diff --git a/.travis.yml b/.travis.yml
index 04da973..89c30ae 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -66,16 +66,16 @@ matrix:
compiler: gcc
# All the trace backends (apart from dtrace)
- env: TARGETS=i386-softmmu,x86_64-softmmu
- EXTRA_CONFIG="--enable-trace-backend=stderr"
+ EXTRA_CONFIG="--enable-trace-backends=stderr"
compiler: gcc
- env: TARGETS=i386-softmmu,x86_64-softmmu
- EXTRA_CONFIG="--enable-trace-backend=simple"
+ EXTRA_CONFIG="--enable-trace-backends=simple"
compiler: gcc
- env: TARGETS=i386-softmmu,x86_64-softmmu
- EXTRA_CONFIG="--enable-trace-backend=ftrace"
+ EXTRA_CONFIG="--enable-trace-backends=ftrace"
TEST_CMD=""
compiler: gcc
- env: TARGETS=i386-softmmu,x86_64-softmmu
EXTRA_PKGS="liblttng-ust-dev liburcu-dev"
- EXTRA_CONFIG="--enable-trace-backend=ust"
+ EXTRA_CONFIG="--enable-trace-backends=ust"
compiler: gcc
diff --git a/Makefile b/Makefile
index a120aab..609bf69 100644
--- a/Makefile
+++ b/Makefile
@@ -52,12 +52,12 @@ GENERATED_HEADERS += trace/generated-events.h
GENERATED_SOURCES += trace/generated-events.c
GENERATED_HEADERS += trace/generated-tracers.h
-ifeq ($(TRACE_BACKEND),dtrace)
+ifeq ($(findstring dtrace,$(TRACE_BACKENDS)),dtrace)
GENERATED_HEADERS += trace/generated-tracers-dtrace.h
endif
GENERATED_SOURCES += trace/generated-tracers.c
-ifeq ($(TRACE_BACKEND),ust)
+ifeq ($(findstring ust,$(TRACE_BACKENDS)),ust)
GENERATED_HEADERS += trace/generated-ust-provider.h
GENERATED_SOURCES += trace/generated-ust.c
endif
diff --git a/Makefile.target b/Makefile.target
index 6d8fde8..749b2e1 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -46,7 +46,7 @@ endif
$(QEMU_PROG).stp-installed: $(SRC_PATH)/trace-events
$(call quiet-command,$(TRACETOOL) \
--format=stap \
- --backend=$(TRACE_BACKEND) \
+ --backends=$(TRACE_BACKENDS) \
--binary=$(bindir)/$(QEMU_PROG) \
--target-name=$(TARGET_NAME) \
--target-type=$(TARGET_TYPE) \
@@ -55,7 +55,7 @@ $(QEMU_PROG).stp-installed: $(SRC_PATH)/trace-events
$(QEMU_PROG).stp: $(SRC_PATH)/trace-events
$(call quiet-command,$(TRACETOOL) \
--format=stap \
- --backend=$(TRACE_BACKEND) \
+ --backends=$(TRACE_BACKENDS) \
--binary=$(realpath .)/$(QEMU_PROG) \
--target-name=$(TARGET_NAME) \
--target-type=$(TARGET_TYPE) \
diff --git a/configure b/configure
index 46bc0c9..73b6539 100755
--- a/configure
+++ b/configure
@@ -180,6 +180,10 @@ path_of() {
return 1
}
+have_backend () {
+echo "$trace_backends" | grep "$1" >/dev/null
+}
+
# default parameters
source_path=`dirname "$0"`
cpu=""
@@ -291,7 +295,7 @@ pkgversion=""
pie=""
zero_malloc=""
qom_cast_debug="yes"
-trace_backend="nop"
+trace_backends="nop"
trace_file="trace"
spice=""
rbd=""
@@ -751,7 +755,7 @@ for opt do
;;
--target-list=*) target_list="$optarg"
;;
- --enable-trace-backend=*) trace_backend="$optarg"
+ --enable-trace-backends=*) trace_backends="$optarg"
;;
--with-trace-file=*) trac
Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
On Fri, 2014-05-09 at 10:38 -0400, Ross Philipson wrote: > >> Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk > >> of the ACPI PCI stuff can be moved there ? > > > > I think it can "shadow" or extend existing DSDT stuff, I don't think it > > can patch as sych. But here we want to dynamically add an entire method > > I think? (or hide, but I don't think that is possible). > > Yea the SSDTs extend the DSDT. The DSDT is loaded to create the name > space and then SSDTs are loaded and added to the name space. If you need > to make runtime modifications like this, it is much easier to do it in > an SSDT as you suggest. What I don't know is whether you could extend > say a device, as in this case, with with a single method in a separate > SSDT. I have never really seen something like that before. So it could be used by having two template SSDTs (one for ejectable, one for not) and outputting the correct ones as necessary? > WRT to hide vs. remove, I believe the intent is to effectively remove > the eject method from a given device by renaming it. It could simply be > removed making the device not eject-able but I think they are trying to > avoid having to recalculate and update the checksum on the DSDT. How does one make the device be not eject-able? I thought it was via the presence or absence of _EJ0 (hence the renaming hack). > As to whether this has to be done at runtime, I don't know. If it does, > I wrote a lib that can generate basically any (rev2) AML on the fly. We > used it e.g. to generate WMI functionality in an SSDT at runtime. I > would be happy to share if it is useful. Thanks. hvmloader seems to generate various bits on the fly based on basic templates already, although perhaps those cases are simpler than this one. Ian.
Re: [Qemu-devel] [PULL 00/38] QMP queue
On Fri, 9 May 2014 12:57:43 +0100 Peter Maydell wrote: > On 8 May 2014 19:52, Luiz Capitulino wrote: > > The following changes since commit 6b342cc9c872e82620fdd32730cd92affa8a19b3: > > > > Merge remote-tracking branch 'remotes/spice/tags/pull-spice-7' into > > staging (2014-05-08 10:57:25 +0100) > > > > are available in the git repository at: > > > > > > git://repo.or.cz/qemu/qmp-unstable.git queue/qmp > > > > for you to fetch changes up to 4c56cbae6908cc597842e86e523886bb8ea8cfb8: > > > > Revert "qapi: Clean up superfluous null check in qapi_dealloc_type_str()" > > (2014-05-08 14:20:26 -0400) > > Hi; this doesn't build for w32, I'm afraid: Can you try again? It should be fixed now. New HEAD is b690d679c1ca65d71b0544a2331d50e9f0f95116 Sorry for that. > > /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c: In function > ‘qga_vss_fsfreeze’: > /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c:160: error: > ‘err’ undeclared (first use in this function) > /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c:160: error: > (Each undeclared identifier is reported only once > /home/petmay01/linaro/qemu-for-merges/qga/vss-win32.c:160: error: for > each function it appears in.) > > thanks > -- PMM >
Re: [Qemu-devel] [PULL 0/4] libcacard-standalone (glib compat & libcacard) series for 2014-05-08
On 8 May 2014 09:30, Michael Tokarev wrote: > Here's a pull request for glib pre-2.31 compatibility layer and libcacard > cleanups on top of it. It has been submitted for review previously: > > v1: http://thread.gmane.org/gmane.comp.emulators.qemu/269432 > v2: http://thread.gmane.org/gmane.comp.emulators.qemu/270259 > > Previous attempt of adding glib compat layer by Stefan Hajnoczi: > http://thread.gmane.org/gmane.comp.emulators.qemu/253830 > > Changes since v2 submission: > > - dropped 3 trivial changes which were applied meantime > - fixed coding style (identation/spacing) in first patch >alevy: I hope these tiny spacing changes are okay for >you to keep your reviewed-by ;) > > Please consider applying. Since this touches several areas of > qemu, there's no clear maintainer so I'm not really sure which > maintainer tree should pick this up. And it isn't very suitable > for -trival either :) I'm still a bit dubious about the approach this patchset takes to glib back-compatibility (ie using #defines etc to fake up the new glib API on older glib versions, rather than defining wrappers), and it sounded from the conversation on IRC today as if Stefan was also not completely convinced. So I don't think there's enough consensus to apply this yet, and I'd rather we had more discussion/review first. thanks -- PMM
Re: [Qemu-devel] [PATCH] qapi: Make the include directive idempotent.
The Friday 09 May 2014 à 07:36:31 (+0200), Markus Armbruster wrote : > Benoît Canet writes: > > > The Thursday 08 May 2014 à 20:30:33 (+0200), Markus Armbruster wrote : > [...] > >> There are two reasons to detect cycles. The technical one is preventing > >> infinite expansion. No longer applies with idempotent include. The > >> other, more abstract one is rejecting nonsensical use of the include > >> directive. I think that one still applies. > [...] > >> > @@ -102,17 +102,16 @@ class QAPISchema: > >> > 'Expected a file name (string), > >> > got: %s' > >> > % include) > >> > include_path = os.path.join(self.input_dir, include) > >> > -if any(include_path == elem[1] > >> > - for elem in self.include_hist): > >> > -raise QAPIExprError(expr_info, "Inclusion loop for > >> > %s" > >> > -% include) > >> > +# make include idempotent by skipping further includes > >> > +if include_path in [x[1] for x in include_hist]: > >> > +continue > >> > >> Loses cycle detection. > > > > It simply also skip cycle includes. If cycle are skipped then cannot do no > > harm. > > Your argument is based exclusively on the technical reason to detect > cycles: cycles need to be caught because they cause infinite recursion. > Since there is no infinite recursion with idempotent include, cycles are > just fine. > > I'm arguing from a more abstract point of view: cycles should be > rejected because they're nonsensical. The fact that they can cause > infinite recursion is an implementation detail. Even without infinite > recursion, they're just as nonsensical as ever. > > [...] > Ok. will redo it.
Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
Il 09/05/2014 16:46, Ian Campbell ha scritto: On Fri, 2014-05-09 at 10:38 -0400, Ross Philipson wrote: Can it be used to patch the DSDT? Or were you (Ian) thinking that the bulk of the ACPI PCI stuff can be moved there ? I think it can "shadow" or extend existing DSDT stuff, I don't think it can patch as sych. But here we want to dynamically add an entire method I think? (or hide, but I don't think that is possible). Yea the SSDTs extend the DSDT. The DSDT is loaded to create the name space and then SSDTs are loaded and added to the name space. If you need to make runtime modifications like this, it is much easier to do it in an SSDT as you suggest. What I don't know is whether you could extend say a device, as in this case, with with a single method in a separate SSDT. I have never really seen something like that before. So it could be used by having two template SSDTs (one for ejectable, one for not) and outputting the correct ones as necessary? WRT to hide vs. remove, I believe the intent is to effectively remove the eject method from a given device by renaming it. It could simply be removed making the device not eject-able but I think they are trying to avoid having to recalculate and update the checksum on the DSDT. How does one make the device be not eject-able? I thought it was via the presence or absence of _EJ0 (hence the renaming hack). As to whether this has to be done at runtime, I don't know. If it does, I wrote a lib that can generate basically any (rev2) AML on the fly. We used it e.g. to generate WMI functionality in an SSDT at runtime. I would be happy to share if it is useful. Thanks. hvmloader seems to generate various bits on the fly based on basic templates already, although perhaps those cases are simpler than this one. Ian. I saw that newer qemu versions generate acpi tables in it. Can be a good idea add the acpi changes needed by xen in qemu instead works on acpi tables also in hvmloader? (and perhaps even avoid unexpected cases with new versions of qemu or similar) If this is a stupid idea sorry for the loss of time. Thanks for any reply and sorry for my bad english.
Re: [Qemu-devel] [PATCH v3 ] trace: Multi-backend tracing
Lluís Vilanova writes:
> Adds support to compile QEMU with multiple tracing backends at the same time.
> For example, you can compile QEMU with:
> $ ./configure --enable-trace-backends=ftrace,dtrace
> Where 'ftrace' can be handy for having an in-flight record of events, and
> 'dtrace' can be later used to extract more information from the system.
> This patch allows having both available without recompiling QEMU.
> Signed-off-by: Lluís Vilanova
> ---
Changes in v3:
* Rebase on 43cbeff.
Changes in v2:
* Rebase on 3e890c7.
* Update ".travis.yml".
* Replace all uses of "TRACE_BACKEND" for "TRACE_BACKENDS".
* Replace "--backend" for "--backends" in "scripts/tracetool.py".
* Update "configure" help and "docs/tracing.txt".
Lluis
> .travis.yml |8 +++--
> Makefile |4 +--
> Makefile.target |4 +--
> configure | 44 +-
> docs/tracing.txt |4 +--
> qemu-io.c |2 +
> scripts/tracetool.py | 41 ++--
> scripts/tracetool/__init__.py | 22 +++
> scripts/tracetool/backend/__init__.py | 15 +-
> trace/Makefile.objs | 22 +++
> trace/control-internal.h |4 +--
> trace/control.c | 49
> -
> trace/control.h | 27 +++---
> trace/default.c | 40 ---
> trace/ftrace.c| 25 +
> trace/ftrace.h|5 +++
> trace/simple.c| 19 +
> trace/simple.h|1 +
> trace/stderr.c| 30
> vl.c |4 +--
> 20 files changed, 144 insertions(+), 226 deletions(-)
> delete mode 100644 trace/default.c
> delete mode 100644 trace/stderr.c
> diff --git a/.travis.yml b/.travis.yml
> index 04da973..89c30ae 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -66,16 +66,16 @@ matrix:
>compiler: gcc
> # All the trace backends (apart from dtrace)
> - env: TARGETS=i386-softmmu,x86_64-softmmu
> - EXTRA_CONFIG="--enable-trace-backend=stderr"
> + EXTRA_CONFIG="--enable-trace-backends=stderr"
>compiler: gcc
> - env: TARGETS=i386-softmmu,x86_64-softmmu
> - EXTRA_CONFIG="--enable-trace-backend=simple"
> + EXTRA_CONFIG="--enable-trace-backends=simple"
>compiler: gcc
> - env: TARGETS=i386-softmmu,x86_64-softmmu
> - EXTRA_CONFIG="--enable-trace-backend=ftrace"
> + EXTRA_CONFIG="--enable-trace-backends=ftrace"
> TEST_CMD=""
>compiler: gcc
> - env: TARGETS=i386-softmmu,x86_64-softmmu
>EXTRA_PKGS="liblttng-ust-dev liburcu-dev"
> - EXTRA_CONFIG="--enable-trace-backend=ust"
> + EXTRA_CONFIG="--enable-trace-backends=ust"
>compiler: gcc
> diff --git a/Makefile b/Makefile
> index a120aab..609bf69 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -52,12 +52,12 @@ GENERATED_HEADERS += trace/generated-events.h
> GENERATED_SOURCES += trace/generated-events.c
> GENERATED_HEADERS += trace/generated-tracers.h
> -ifeq ($(TRACE_BACKEND),dtrace)
> +ifeq ($(findstring dtrace,$(TRACE_BACKENDS)),dtrace)
> GENERATED_HEADERS += trace/generated-tracers-dtrace.h
> endif
> GENERATED_SOURCES += trace/generated-tracers.c
> -ifeq ($(TRACE_BACKEND),ust)
> +ifeq ($(findstring ust,$(TRACE_BACKENDS)),ust)
> GENERATED_HEADERS += trace/generated-ust-provider.h
> GENERATED_SOURCES += trace/generated-ust.c
> endif
> diff --git a/Makefile.target b/Makefile.target
> index 6d8fde8..749b2e1 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -46,7 +46,7 @@ endif
> $(QEMU_PROG).stp-installed: $(SRC_PATH)/trace-events
> $(call quiet-command,$(TRACETOOL) \
> --format=stap \
> - --backend=$(TRACE_BACKEND) \
> + --backends=$(TRACE_BACKENDS) \
> --binary=$(bindir)/$(QEMU_PROG) \
> --target-name=$(TARGET_NAME) \
> --target-type=$(TARGET_TYPE) \
> @@ -55,7 +55,7 @@ $(QEMU_PROG).stp-installed: $(SRC_PATH)/trace-events
> $(QEMU_PROG).stp: $(SRC_PATH)/trace-events
> $(call quiet-command,$(TRACETOOL) \
> --format=stap \
> - --backend=$(TRACE_BACKEND) \
> + --backends=$(TRACE_BACKENDS) \
> --binary=$(realpath .)/$(QEMU_PROG) \
> --target-name=$(TARGET_NAME) \
> --target-type=$(TARGET_TYPE) \
> diff --git a/configure b/configure
> index 46bc0c9..73b6539 100755
> --- a/configure
> +++ b/configure
> @@ -180,6 +180,10 @@ path_of() {
> return 1
> }
> +have_backend () {
> +echo
Re: [Qemu-devel] [PATCH] qapi: Make the include directive idempotent.
Markus Armbruster writes: > Benoît Canet writes: >> The Thursday 08 May 2014 à 20:30:33 (+0200), Markus Armbruster wrote : > [...] >>> There are two reasons to detect cycles. The technical one is preventing >>> infinite expansion. No longer applies with idempotent include. The >>> other, more abstract one is rejecting nonsensical use of the include >>> directive. I think that one still applies. > [...] >>> > @@ -102,17 +102,16 @@ class QAPISchema: >>> > 'Expected a file name (string), >>> > got: %s' >>> > % include) >>> > include_path = os.path.join(self.input_dir, include) >>> > -if any(include_path == elem[1] >>> > - for elem in self.include_hist): >>> > -raise QAPIExprError(expr_info, "Inclusion loop for >>> > %s" >>> > -% include) >>> > +# make include idempotent by skipping further includes >>> > +if include_path in [x[1] for x in include_hist]: >>> > +continue >>> >>> Loses cycle detection. >> >> It simply also skip cycle includes. If cycle are skipped then cannot do no >> harm. > Your argument is based exclusively on the technical reason to detect > cycles: cycles need to be caught because they cause infinite recursion. > Since there is no infinite recursion with idempotent include, cycles are > just fine. > I'm arguing from a more abstract point of view: cycles should be > rejected because they're nonsensical. The fact that they can cause > infinite recursion is an implementation detail. Even without infinite > recursion, they're just as nonsensical as ever. > [...] I agree, a cycle is not the same as a double definition due to a file being included multiple times: 1) main.json: include bar.json bar.json : include baz.json baz.json : include bar.json 2) main.json: include bar.json include baz.json bar.json : include common.json ... baz.json : include common.json ... common.json: The first one is obviously wrong (there's a cycle), while the second one needs the idempotency feature to avoid doubly defining the contents in common.json. Another question is whether keeping cycle detection as an error is worth the coding effort. It should not be very complex if you keep a global (as in this patch) and a local (as I did) inclusion history. Lluis -- "And it's much the same thing with knowledge, for whenever you learn something new, the whole world becomes that much richer." -- The Princess of Pure Reason, as told by Norton Juster in The Phantom Tollbooth
Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching
On Fri, 2014-05-09 at 16:56 +0200, Fabio Fantoni wrote: > Il 09/05/2014 16:46, Ian Campbell ha scritto: > > On Fri, 2014-05-09 at 10:38 -0400, Ross Philipson wrote: > Can it be used to patch the DSDT? Or were you (Ian) thinking that the > bulk > of the ACPI PCI stuff can be moved there ? > >>> I think it can "shadow" or extend existing DSDT stuff, I don't think it > >>> can patch as sych. But here we want to dynamically add an entire method > >>> I think? (or hide, but I don't think that is possible). > >> Yea the SSDTs extend the DSDT. The DSDT is loaded to create the name > >> space and then SSDTs are loaded and added to the name space. If you need > >> to make runtime modifications like this, it is much easier to do it in > >> an SSDT as you suggest. What I don't know is whether you could extend > >> say a device, as in this case, with with a single method in a separate > >> SSDT. I have never really seen something like that before. > > So it could be used by having two template SSDTs (one for ejectable, one > > for not) and outputting the correct ones as necessary? > > > >> WRT to hide vs. remove, I believe the intent is to effectively remove > >> the eject method from a given device by renaming it. It could simply be > >> removed making the device not eject-able but I think they are trying to > >> avoid having to recalculate and update the checksum on the DSDT. > > How does one make the device be not eject-able? I thought it was via the > > presence or absence of _EJ0 (hence the renaming hack). > > > >> As to whether this has to be done at runtime, I don't know. If it does, > >> I wrote a lib that can generate basically any (rev2) AML on the fly. We > >> used it e.g. to generate WMI functionality in an SSDT at runtime. I > >> would be happy to share if it is useful. > > Thanks. hvmloader seems to generate various bits on the fly based on > > basic templates already, although perhaps those cases are simpler than > > this one. > > > > Ian. > > > > I saw that newer qemu versions generate acpi tables in it. > Can be a good idea add the acpi changes needed by xen in qemu instead > works on acpi tables also in hvmloader? (and perhaps even avoid > unexpected cases with new versions of qemu or similar) I already responded to a similar suggestion on a previous iteration of this patch. > If this is a stupid idea sorry for the loss of time. > > Thanks for any reply and sorry for my bad english.
[Qemu-devel] [PATCH] qemu-img fails to delete last snapshot
When deleting the last snapshot, copying the resulting snapshot table
currently fails, causing the delete operation to also fail. Fix the
failure by skipping the copy and just writing the snapshot header and
freeing the extra clusters.
Signed-off-by: Mike Day
---
There are two specific problems in the curent code. First is a lack of
parenthesis in the calculation of a memmove parameter:
s->nb_snapshots - snapshot_index - 1
When s->nb_snapshots is 0, snapshot_index is 1.
0 - 1 - 1 = 0xfffe
it should be:
0 - (1 - 1) = 0x00
The second problem is shifting the snapshot table to the left. After
removing the last snapshot there are no existing snapshots to be
shifted. All that needs to be done is to write the header and
unallocate the blocks.
block/qcow2-snapshot.c | 25 +++--
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 0aa9def..c8b842c 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -165,9 +165,11 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
assert(offset <= INT_MAX);
snapshots_size = offset;
-
/* Allocate space for the new snapshot list */
-snapshots_offset = qcow2_alloc_clusters(bs, snapshots_size);
+snapshots_offset = 0;
+if (snapshots_size) {
+snapshots_offset = qcow2_alloc_clusters(bs, snapshots_size);
+}
offset = snapshots_offset;
if (offset < 0) {
ret = offset;
@@ -180,12 +182,13 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
/* The snapshot list position has not yet been updated, so these clusters
* must indeed be completely free */
-ret = qcow2_pre_write_overlap_check(bs, 0, offset, snapshots_size);
-if (ret < 0) {
-goto fail;
+if (snapshots_size) {
+ret = qcow2_pre_write_overlap_check(bs, 0, offset, snapshots_size);
+if (ret < 0) {
+goto fail;
+}
}
-
/* Write all snapshots to the new list */
for(i = 0; i < s->nb_snapshots; i++) {
sn = s->snapshots + i;
@@ -590,12 +593,14 @@ int qcow2_snapshot_delete(BlockDriverState *bs,
return -ENOENT;
}
sn = s->snapshots[snapshot_index];
-
/* Remove it from the snapshot list */
-memmove(s->snapshots + snapshot_index,
-s->snapshots + snapshot_index + 1,
-(s->nb_snapshots - snapshot_index - 1) * sizeof(sn));
s->nb_snapshots--;
+if (s->nb_snapshots) {
+memmove(s->snapshots + snapshot_index,
+s->snapshots + snapshot_index + 1,
+(s->nb_snapshots - (snapshot_index - 1)) * sizeof(sn));
+}
+
ret = qcow2_write_snapshots(bs);
if (ret < 0) {
error_setg_errno(errp, -ret,
--
1.9.0
[Qemu-devel] uvesafb doesn't work with seabios
Hello,
I'm using QEmu 2.0.0 on a Linux host (x86-64) with a quite special
target system that uses uvesafb ('video=uvesafb:1024x768-32'). I get
following errors in the target system:
uvesafb: Getting mode info block for mode 0x2 failed (eax=0x4f01, err=1)
uvesafb: Getting mode info block for mode 0x3 failed (eax=0x4f01, err=1)
uvesafb: Getting mode info block for mode 0x4 failed (eax=0x4f01, err=1)
uvesafb: Getting mode info block for mode 0x5 failed (eax=0x4f01, err=1)
uvesafb: Getting mode info block for mode 0x6 failed (eax=0x4f01, err=1)
uvesafb: Getting mode info block for mode 0x7 failed (eax=0x4f01, err=1)
uvesafb: Getting mode info block for mode 0xd failed (eax=0x4f01, err=1)
uvesafb: Getting mode info block for mode 0xe failed (eax=0x4f01, err=1)
uvesafb: Getting mode info block for mode 0xf failed (eax=0x4f01, err=1)
uvesafb: Getting mode info block for mode 0x10 failed (eax=0x4f01, err=1)
uvesafb: Getting mode info block for mode 0x11 failed (eax=0x4f01, err=1)
uvesafb: Getting mode info block for mode 0x12 failed (eax=0x4f01, err=1)
uvesafb: Getting mode info block for mode 0x13 failed (eax=0x4f01, err=1)
uvesafb: Getting mode info block for mode 0x6a failed (eax=0x4f01, err=1)
uvesafb: vbe_init() failed with -22
uvesafb: probe of uvesafb.0 failed with error -22
With QEmu 1.6, it worked. I bisected that down to following commit:
commit 6eefccc0bb9c34051b1e21880fc3a1c1c8686edd
Author: Gerd Hoffmann
Date: Mon Dec 2 13:01:20 2013 +0100
roms: update vgabios binaries
And indeed, when I switch 'vgabios-stdvga.bin' to the old variant, it works.
What can I do to track down the problem? I tried to build the svga bios
inside the QEMU tree, but I didn't succeed. Building it in the current
seabios git tree worked, but I got a different error...
Regards,
Bernhard
[Qemu-devel] [PATCH] bsd-user: Remove reference to CONFIG_UNAME_RELEASE
Commit e586822a5 broke the bsd-user build when it removed the CONFIG_UNAME_RELEASE define but forgot to remove the use of it in bsd-user. Fix this in the simplest possible way (bsd-user doesn't make any use at all of the qemu_uname_release variable except to allow it to be pointlessly set by the user, so this is all we need to do.) Signed-off-by: Peter Maydell --- Not even compile tested, since I don't have a BSD system to hand. If somebody would like to provide a Tested-by: then I'll apply it to master... bsd-user/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bsd-user/main.c b/bsd-user/main.c index f81ba55..0f9169d 100644 --- a/bsd-user/main.c +++ b/bsd-user/main.c @@ -43,7 +43,7 @@ unsigned long reserved_va; #endif static const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX; -const char *qemu_uname_release = CONFIG_UNAME_RELEASE; +const char *qemu_uname_release; extern char **environ; enum BSDType bsd_type; -- 1.9.2
[Qemu-devel] [PATCH v2] qcow2: Patch for shrinking qcow2 disk image
As the realization of raw shrinking, so when do qcow2 shrinking, do not
check l1 entries. When resize to size1(size1 < "disk size"), the custemer
knows this will destory the data. So no need to check the l1 entries which
is used or not.
This is v2 of the original patch. thx.
Signed-off-by: Jun Li
---
block/qcow2-cluster.c | 17 -
block/qcow2-snapshot.c | 2 +-
block/qcow2.c | 10 ++
block/qcow2.h | 4 ++--
4 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 76d2bcf..8fbbf7f 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -29,8 +29,8 @@
#include "block/qcow2.h"
#include "trace.h"
-int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
-bool exact_size)
+int qcow2_truncate_l1_table(BlockDriverState *bs, uint64_t min_size,
+bool exact_size)
{
BDRVQcowState *s = bs->opaque;
int new_l1_size2, ret, i;
@@ -39,8 +39,9 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t
min_size,
int64_t new_l1_table_offset, new_l1_size;
uint8_t data[12];
-if (min_size <= s->l1_size)
+if (min_size == s->l1_size) {
return 0;
+}
/* Do a sanity check on min_size before trying to calculate new_l1_size
* (this prevents overflows during the while loop for the calculation of
@@ -73,7 +74,13 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t
min_size,
new_l1_size2 = sizeof(uint64_t) * new_l1_size;
new_l1_table = g_malloc0(align_offset(new_l1_size2, 512));
-memcpy(new_l1_table, s->l1_table, s->l1_size * sizeof(uint64_t));
+
+/* shrinking or growing l1 table */
+if (min_size < s->l1_size) {
+memcpy(new_l1_table, s->l1_table, new_l1_size2);
+} else {
+memcpy(new_l1_table, s->l1_table, s->l1_size * sizeof(uint64_t));
+}
/* write new table (align to cluster) */
BLKDBG_EVENT(bs->file, BLKDBG_L1_GROW_ALLOC_TABLE);
@@ -558,7 +565,7 @@ static int get_cluster_table(BlockDriverState *bs, uint64_t
offset,
l1_index = offset >> (s->l2_bits + s->cluster_bits);
if (l1_index >= s->l1_size) {
-ret = qcow2_grow_l1_table(bs, l1_index + 1, false);
+ret = qcow2_truncate_l1_table(bs, l1_index + 1, false);
if (ret < 0) {
return ret;
}
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 0aa9def..6ba460e 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -483,7 +483,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char
*snapshot_id)
* L1 table of the snapshot. If the snapshot L1 table is smaller, the
* current one must be padded with zeros.
*/
-ret = qcow2_grow_l1_table(bs, sn->l1_size, true);
+ret = qcow2_truncate_l1_table(bs, sn->l1_size, true);
if (ret < 0) {
goto fail;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index a4b97e8..70f951c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1877,7 +1877,7 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t
offset)
int64_t new_l1_size;
int ret;
-if (offset & 511) {
+if (offset <= 0 || offset & 511) {
error_report("The new size must be a multiple of 512");
return -EINVAL;
}
@@ -1888,14 +1888,8 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t
offset)
return -ENOTSUP;
}
-/* shrinking is currently not supported */
-if (offset < bs->total_sectors * 512) {
-error_report("qcow2 doesn't support shrinking images yet");
-return -ENOTSUP;
-}
-
new_l1_size = size_to_l1(s, offset);
-ret = qcow2_grow_l1_table(bs, new_l1_size, true);
+ret = qcow2_truncate_l1_table(bs, new_l1_size, true);
if (ret < 0) {
return ret;
}
diff --git a/block/qcow2.h b/block/qcow2.h
index b49424b..fa36930 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -499,8 +499,8 @@ int qcow2_pre_write_overlap_check(BlockDriverState *bs, int
ign, int64_t offset,
int64_t size);
/* qcow2-cluster.c functions */
-int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
-bool exact_size);
+int qcow2_truncate_l1_table(BlockDriverState *bs, uint64_t min_size,
+bool exact_size);
int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index);
void qcow2_l2_cache_reset(BlockDriverState *bs);
int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset);
--
1.8.3.1
Re: [Qemu-devel] [PATCH 4] block/raw-posix: Try both FIEMAP and SEEK_HOLE
On Thu, May 08, 2014 at 08:57:55PM +0200, Max Reitz wrote: > The current version of raw-posix always uses ioctl(FS_IOC_FIEMAP) if > FIEMAP is available; lseek with SEEK_HOLE/SEEK_DATA are not even > compiled in in this case. However, there may be implementations which > support the latter but not the former (e.g., NFSv4.2) as well as vice > versa. > > To cover both cases, try FIEMAP first (as this will return -ENOTSUP if > not supported instead of returning a failsafe value (everything > allocated as a single extent)) and if that does not work, fall back to > SEEK_HOLE/SEEK_DATA. > > Signed-off-by: Max Reitz > --- > v4: > - If the order of FIEMAP and SEEK_HOLE are reversed (that is, kept the >way they are now), the tristates from v3 are not required anymore, as >FIEMAP can never be in an “unknown” state and the worst that >SEEK_HOLE can do is to report everything as allocated (which is the >final fallback anyway). This patch is thus basically the same as v2, >only with the order of FIEMAP and SEEK_HOLE reversed (first FIEMAP, >then SEEK_HOLE). [Paolo] > --- > block/raw-posix.c | 127 > +- > 1 file changed, 77 insertions(+), 50 deletions(-) Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
Re: [Qemu-devel] [PATCH 0/5] Clean up around bdrv_getlength()
On Fri, May 09, 2014 at 11:48:13AM +0200, Markus Armbruster wrote: > Issues addressed in this series: > > * BlockDriver method bdrv_getlength() generally returns -errno, but > some implementations return -1 instead. Fix them [PATCH 1]. > > * Frequent conversions between sectors and bytes complicate the code > needlessly. Clean up some [PATCH 2+3]. > > * bdrv_getlength() always returns a multiple of BDRV_SECTOR_SIZE, but > some places appear to be confused about that, and align the result > up or down. Don't [PATCH 4]. > > * bdrv_get_geometry() hides errors. Don't use it in places where > errors should be detected [PATCH 5]. > > Issues not addressed: > > * There are quite a few literals left in the code where > BDRV_SECTOR_SIZE, BDRV_SECTOR_BITS or BDRV_SECTOR_MASK should be > used instead. > > * Error handling is missing in places, but it's not always obvious > whether errors can actually happen, and if yes, how to handle them. > > Markus Armbruster (5): > raw-posix: Fix raw_getlength() to always return -errno on error > block: New bdrv_nb_sectors() > block: Use bdrv_nb_sectors() when sectors, not bytes are wanted There's a regression in this patch. Dropped from the block queue because qemu-iotests ./check -qcow2 030 seems to go into an infinite loop or at least takes a long time. Stefan
[Qemu-devel] [PATCH V2] Let the C generated bits of QAPI be generated only once when identical includes are done
in V2: Keep the cycle detection [Markus] Don't talk about idempottent [Markus] Benoît Canet (1): qapi: Let redundant includes be skipped excepted the first occurrence. scripts/qapi.py | 14 +++--- tests/Makefile |3 ++- tests/qapi-schema/include-only-one-time.exit |1 + tests/qapi-schema/include-only-one-time.json |3 +++ tests/qapi-schema/include-only-one-time.out |3 +++ tests/qapi-schema/sub-include-only-one-time.json |2 ++ 6 files changed, 22 insertions(+), 4 deletions(-) create mode 100644 tests/qapi-schema/include-only-one-time.err create mode 100644 tests/qapi-schema/include-only-one-time.exit create mode 100644 tests/qapi-schema/include-only-one-time.json create mode 100644 tests/qapi-schema/include-only-one-time.out create mode 100644 tests/qapi-schema/sub-include-only-one-time.json -- 1.7.10.4
[Qemu-devel] [PATCH V2] qapi: Let redundant includes be skipped excepted the first occurrence.
The purpose of this change is to help create a json file containing
common definitions; each bit of generated C code must be spitted
only one time.
A second history global to all QAPISchema instances has been added
to detect when a file is included more than one time and skip these
includes.
It does not act as a stack and the changes made to it by the
__init__ function are propagated back to the caller so it's really
a global state.
Signed-off-by: Benoit Canet
---
scripts/qapi.py | 14 +++---
tests/Makefile |3 ++-
tests/qapi-schema/include-only-one-time.exit |1 +
tests/qapi-schema/include-only-one-time.json |3 +++
tests/qapi-schema/include-only-one-time.out |3 +++
tests/qapi-schema/sub-include-only-one-time.json |2 ++
6 files changed, 22 insertions(+), 4 deletions(-)
create mode 100644 tests/qapi-schema/include-only-one-time.err
create mode 100644 tests/qapi-schema/include-only-one-time.exit
create mode 100644 tests/qapi-schema/include-only-one-time.json
create mode 100644 tests/qapi-schema/include-only-one-time.out
create mode 100644 tests/qapi-schema/sub-include-only-one-time.json
diff --git a/scripts/qapi.py b/scripts/qapi.py
index ec806aa..0265b40 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -73,13 +73,18 @@ class QAPIExprError(Exception):
class QAPISchema:
-def __init__(self, fp, input_relname=None, include_hist=[],
parent_info=None):
+def __init__(self, fp, input_relname=None, include_hist=[],
+ previously_included=[], parent_info=None):
+""" include_hist is a stack used to detect inclusion cycles
+previously_included is a global state used to avoid multiple
+inclusions of the same file"""
input_fname = os.path.abspath(fp.name)
if input_relname is None:
input_relname = fp.name
self.input_dir = os.path.dirname(input_fname)
self.input_file = input_relname
self.include_hist = include_hist + [(input_relname, input_fname)]
+previously_included.append(input_fname)
self.parent_info = parent_info
self.src = fp.read()
if self.src == '' or self.src[-1] != '\n':
@@ -106,13 +111,16 @@ class QAPISchema:
for elem in self.include_hist):
raise QAPIExprError(expr_info, "Inclusion loop for %s"
% include)
+# skip multiple include of the same file
+if include_path in previously_included:
+continue
try:
fobj = open(include_path, 'r')
except IOError as e:
raise QAPIExprError(expr_info,
'%s: %s' % (e.strerror, include))
-exprs_include = QAPISchema(fobj, include,
- self.include_hist, expr_info)
+exprs_include = QAPISchema(fobj, include, self.include_hist,
+ previously_included, expr_info)
self.exprs.extend(exprs_include.exprs)
else:
expr_elem = {'expr': expr,
diff --git a/tests/Makefile b/tests/Makefile
index a8d45be..a0f12fb 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -182,7 +182,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
flat-union-string-discriminator.json \
include-simple.json include-relpath.json include-format-err.json \
include-non-file.json include-no-file.json include-before-err.json \
-include-nested-err.json include-self-cycle.json include-cycle.json)
+include-nested-err.json include-self-cycle.json include-cycle.json \
+include-only-one-time.json)
GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h
tests/test-qmp-commands.h
diff --git a/tests/qapi-schema/include-only-one-time.err
b/tests/qapi-schema/include-only-one-time.err
new file mode 100644
index 000..e69de29
diff --git a/tests/qapi-schema/include-only-one-time.exit
b/tests/qapi-schema/include-only-one-time.exit
new file mode 100644
index 000..573541a
--- /dev/null
+++ b/tests/qapi-schema/include-only-one-time.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/include-only-one-time.json
b/tests/qapi-schema/include-only-one-time.json
new file mode 100644
index 000..11772e3
--- /dev/null
+++ b/tests/qapi-schema/include-only-one-time.json
@@ -0,0 +1,3 @@
+{ 'include': 'comments.json' }
+{ 'include': 'sub-include-only-one-time.json' }
+{ 'include': 'comments.json' }
diff --git a/tests/qapi-schema/include-only-one-time.out
b/tests/qapi-schema/include-only-one-time.out
new file mode 100644
index 000..4ce3dcf
--- /dev/null
+++ b/tests/qapi-schema/include-only-one-time.out
@@ -0,0 +1,3 @@
+[OrderedDict([('enum', 'Status'), ('data',
Re: [Qemu-devel] [PULL 00/38] QMP queue
On 9 May 2014 14:50, Luiz Capitulino wrote: > On Fri, 9 May 2014 12:57:43 +0100 > Peter Maydell wrote: > >> On 8 May 2014 19:52, Luiz Capitulino wrote: >> > The following changes since commit >> > 6b342cc9c872e82620fdd32730cd92affa8a19b3: >> > >> > Merge remote-tracking branch 'remotes/spice/tags/pull-spice-7' into >> > staging (2014-05-08 10:57:25 +0100) >> > >> > are available in the git repository at: >> > >> > >> > git://repo.or.cz/qemu/qmp-unstable.git queue/qmp >> > >> > for you to fetch changes up to 4c56cbae6908cc597842e86e523886bb8ea8cfb8: >> > >> > Revert "qapi: Clean up superfluous null check in >> > qapi_dealloc_type_str()" (2014-05-08 14:20:26 -0400) >> >> Hi; this doesn't build for w32, I'm afraid: > > Can you try again? It should be fixed now. New HEAD is > b690d679c1ca65d71b0544a2331d50e9f0f95116 > > Sorry for that. No problem; applied this version, thanks. -- PMM
