Re: [Qemu-devel] [Bug 893956] Re: qemu-img bug with dynamic vhd
On Tue, Sep 4, 2012 at 4:00 PM, Serge Hallyn 893...@bugs.launchpad.net wrote: Though that commit and the comments were about 127G images. HIs is only 27G. The 127 GB limit applies to the virtual disk size, not to the size of the image file itself. Also, 'qemu-img info' is also showing the error, which shows that this is not being done on vpc_create(). I'm not sure what you mean. Your commit (efc8243d00) added an -EFBIG return to vcp_open(), not vpc_create(). This will affect qemu-img info. We need the header/footer contents to be sure what's going on here. Stefan
Re: [Qemu-devel] boot device order has no effect for virtio-scsi devices
Il 05/09/2012 00:13, ching ha scritto: Ah, ok. libvirt for now supports only booting from LUN 0. You can use ^^^ Oops, should have been SeaBIOS. multiple targets instead of multiple units. Paolo it works. thanks a lot. hope that libvirt will document it somewhere Please raise this issue on the libvirt lists, then. actually, why is the problem related to libvirt, not qemu, nor seabios? libvirt has generated the qemu command line with bootindex already. Yes, it's SeaBIOS. It is documented in the code. Paolo
Re: [Qemu-devel] [Bug 893956] Re: qemu-img bug with dynamic vhd
On Mon, Sep 3, 2012 at 10:41 AM, franxico 893...@bugs.launchpad.net wrote:
Hi,
I'm having the same problem. I'm using qemu-img 1.0, running from a
Ubuntu Server 12.04 x64 on a SW RAID, ext4.
The .VHD has 29GB and was made using disk2vhd.
Here is the command and the results:
sudo kvm-img convert -f vpc -O raw image.VHD image.img
[sudo] password for sysop:
kvm-img: Could not open 'image.VHD': File too large
kvm-img: Could not open 'image.VHD'
Same error doing a simple qemu-img info image.vhd
Please post the output of the following commands:
$ hexdump -C -n 512 image.VHD
$ hexdump -C -n 512 -s $(($(ls -l image.VHD | awk '{ print $5 }') -
512)) image.VHD
This will show the file header/footer, which contains fields that are
validated when opening the file.
Thanks,
Stefan
Re: [Qemu-devel] VM Communications inner a network bridge
On Tue, Sep 4, 2012 at 1:50 PM, GaoYi gaoyi...@gmail.com wrote: When 2 VMs are started on the same network bridge, will the TCP/IP packets between them still go through the router (actually these 2 VMs share the same physical network card)? If so, is it possible to just pass the packets inner the network card? If you have 2 VMs using tap network interfaces on a Linux software bridge, then packets will be passed between the tap interfaces. Packets will not go onto the host's physical interface (e.g. eth0). This is regular Ethernet bridge behavior: the bridge decides on which port(s) to forward the packet based on its MAC address to port mapping. Stefan
Re: [Qemu-devel] [PATCH v2 2/7] s390/kvm: Add support for machine checks.
On Tue, Sep 04, 2012 at 05:13:25PM +0200, Cornelia Huck wrote:
Just some quick comments:
[...]
int kvm_s390_inject_program_int(struct kvm_vcpu *vcpu, u16 code)
{
struct kvm_s390_local_interrupt *li = vcpu-arch.local_int;
@@ -648,6 +747,12 @@ int kvm_s390_inject_vm(struct kvm *kvm,
case KVM_S390_INT_EMERGENCY:
kfree(inti);
return -EINVAL;
+ case KVM_S390_MCHK:
+ VM_EVENT(kvm, 5, inject: machine check parm64:%llx,
+ s390int-parm64);
+ inti-type = s390int-type;
+ inti-mchk.mcic = s390int-parm64;
+ break;
The kvm_s390_interrupt struct seems to be inappropriate to pass machine check
data around.
E.g. if you want to inject an uncorrectable storage error, because the host
failed to swap in a page, you must also pass a failing storage address which
doesn't fit into this structure.
Just something you should consider. ;)
+static int handle_lpswe(struct kvm_vcpu *vcpu)
+{
+ int base2 = vcpu-arch.sie_block-ipb 28;
+ int disp2 = ((vcpu-arch.sie_block-ipb 0x0fff) 16);
Sooner or later we need helper functions which extract the significant parts
of an instruction.
Maybay something like insn_[type]_get_base2(...) or simply structures like
struct insn_[type], which allow to easily access parts of an instruction.
+ u64 addr;
+ u64 new_psw[2];
psw_t?
+
+ addr = disp2;
+ if (base2)
+ addr += vcpu-run-s.regs.gprs[base2];
+
+ if (addr 7) {
+ kvm_s390_inject_program_int(vcpu, PGM_SPECIFICATION);
+ goto out;
+ }
+
+ if (copy_from_guest(vcpu, new_psw, addr, sizeof(*new_psw))) {
I assume that should be sizeof(new_psw). Did that ever work?!
+ if ((vcpu-arch.sie_block-gpsw.mask 0xb80800fe7fff) ||
+ (((vcpu-arch.sie_block-gpsw.mask 0x00011000) ==
+ 0x1000)
+ (vcpu-arch.sie_block-gpsw.addr 0x8000)) ||
+ (!(vcpu-arch.sie_block-gpsw.mask 0x00018000)
+ (vcpu-arch.sie_block-gpsw.addr 0xfff0)) ||
+ ((vcpu-arch.sie_block-gpsw.mask 0x00011000) ==
+ 0x0001)) {
This is not very readable...
Please make use of the PSW defines in ptrace.h and add new ones if needed.
Also please make use of (move) the PSW32 defines in compat.h.
Re: [Qemu-devel] [PATCH v2 1/7] s390/kvm: Support for I/O interrupts.
On 09/04/2012 06:13 PM, Cornelia Huck wrote:
Add support for handling I/O interrupts (standard, subchannel-related
ones and rudimentary adapter interrupts).
The subchannel-identifying parameters are encoded into the interrupt
type.
I/O interrupts are floating, so they can't be injected on a specific
vcpu.
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index d808694..5a36e65 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -396,6 +396,12 @@ struct kvm_s390_psw {
#define KVM_S390_INT_SERVICE 0x2401u
#define KVM_S390_INT_EMERGENCY 0x1201u
#define KVM_S390_INT_EXTERNAL_CALL 0x1202u
+#define KVM_S390_INT_IO(ai,cssid,ssid,schid) \
+ (((schid)) | \
+ ((ssid) 16) | \
+ ((cssid) 18) | \
+ ((ai) 26))
+
Documentation?
--
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH v6 2/2] block: Support GlusterFS as a QEMU block backend
On Thu, Aug 09, 2012 at 06:32:16PM +0530, Bharata B Rao wrote:
+static void qemu_gluster_complete_aio(GlusterAIOCB *acb)
+{
+int ret;
+
+if (acb-canceled) {
+qemu_aio_release(acb);
+return;
+}
+
+if (acb-ret == acb-size) {
+ret = 0; /* Success */
+} else if (acb-ret 0) {
+ret = acb-ret; /* Read/Write failed */
+} else {
+ret = -EIO; /* Partial read/write - fail it */
+}
+acb-common.cb(acb-common.opaque, ret);
The .cb() here is bdrv_co_io_em_complete(). It does qemu_coroutine_enter(),
handles the return value and comes back here.
But if the bdrv_read or bdrv_write or bdrv_flush was called from a
coroutine context (as against they themselves creating a new coroutine),
the above .cb() call above doesn't return to this point. Hence I won't
be able to release the acb and decrement the qemu_aio_count.
What could be the issue here ? In general, how do I ensure that my
aio calls get completed correctly in such scenarios where bdrv_read etc
are called from coroutine context rather than from main thread context ?
Creating qcow2 image would lead to this scenario where
-bdrv_create (=qcow2_create) will create a coroutine and subsequently
read and write are called within qcow2_create in coroutine context itself.
+qemu_aio_release(acb);
+}
+
+static void qemu_gluster_aio_event_reader(void *opaque)
+{
+BDRVGlusterState *s = opaque;
+GlusterAIOCB *event_acb;
+int event_reader_pos = 0;
+ssize_t ret;
+
+do {
+char *p = (char *)event_acb;
+
+ret = read(s-fds[GLUSTER_FD_READ], p + event_reader_pos,
+ sizeof(event_acb) - event_reader_pos);
+if (ret 0) {
+event_reader_pos += ret;
+if (event_reader_pos == sizeof(event_acb)) {
+event_reader_pos = 0;
+qemu_gluster_complete_aio(event_acb);
+s-qemu_aio_count--;
+}
+}
+} while (ret 0 errno == EINTR);
+}
+
Re: [Qemu-devel] [RFC PATCH v2 0/7] s390: virtual css host support.
On 09/04/2012 06:13 PM, Cornelia Huck wrote: Hi, here's the second revision of the virtual channel subsystem support for s390. I changed the representation of the channel subsystem, introducing channel subsystem images, which brings it closer to the actual implementation. A new ioctl for adding a new channel subsystem image has also been introduced. Looks good to me, though of course I could only do a shallow review. I'd like to see acks from an s390 maintainer on the non-kvm patches, and will apply after the other comments are addressed. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem
[...] We could externalize the refcounting and push it into device code. This means: memory_region_init_io(s-mem, dev) ... object_ref(dev) memory_region_add_subregion(..., dev-mr) ... memory_region_del_subregion(..., dev-mr) // implied flush object_unref(dev) I think object_ref(dev) just another style to push MemoryRegionOps::object() to device level. And I think we can bypass it. The caller (unplug, pci-reconfig ) of memory_region_del_subregion() ensure the @dev is valid. If the implied flush is implemented in synchronize, _del_subregion() will guarantee no reader for @dev-mr and @dev exist any longer. The above code has a deadlock. memory_region_del_subregion() may be called under the device lock (since it may be the result of mmio to the device), and if the flush uses synchronized_rcu(), it will wait forever for the read-side critical section to complete. But if _del_subregion() just wait for mr-X quiescent period, while calling in mr-Y's read side critical section, then we can avoid deadlock. I saw in pci-mapping, we delete mr-X in mr-Y read side. So I think we can save both object_ref/unref(dev) for memory system. The only problem is that whether we can implement it as synchronous or not, is it possible that we can launch a _del_subregion(mr-X) in mr-X's dispatcher? Yes. Real cases exist. Oh, I find the sample code, then, the deadlock is unavoidable in this method. What alternatives remain? I think a way out may be async+refcnt If we consider the relationship of MemoryRegionImpl and device as the one between file and file-private_data in Linux. Then the creation of impl will object_ref(dev) and when impl-ref=0, it will object_unref(dev) But this is an async model, for those client which need to know the end of flush, we can adopt callback just like call_rcu(). During this thread, it seems that no matter we adopt refcnt on MemoryRegionImpl or not, protecting MemoryRegion::opaque during dispatch is still required. It is challenging to substitute memory_region_init_io() 's 3rd parameter from void* to Object*, owning to the conflict that life-cycle management need the host of opaque, while Object and mr need 1:1 map. So I think, I will move forward on the way based on MemoryRegionOps::object(). Right? Regards, pingfan Regards, pingfan -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH v2 1/7] s390/kvm: Support for I/O interrupts.
On Wed, 05 Sep 2012 10:28:53 +0300
Avi Kivity a...@redhat.com wrote:
On 09/04/2012 06:13 PM, Cornelia Huck wrote:
Add support for handling I/O interrupts (standard, subchannel-related
ones and rudimentary adapter interrupts).
The subchannel-identifying parameters are encoded into the interrupt
type.
I/O interrupts are floating, so they can't be injected on a specific
vcpu.
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index d808694..5a36e65 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -396,6 +396,12 @@ struct kvm_s390_psw {
#define KVM_S390_INT_SERVICE 0x2401u
#define KVM_S390_INT_EMERGENCY 0x1201u
#define KVM_S390_INT_EXTERNAL_CALL 0x1202u
+#define KVM_S390_INT_IO(ai,cssid,ssid,schid) \
+ (((schid)) | \
+((ssid) 16) | \
+((cssid) 18) | \
+((ai) 26))
+
Documentation?
KVM_S390_INTERRUPT is currently completely undocumented... I'll
probably do an extra patch and then have this and the machine check
patch add the new values.
[Qemu-devel] [PATCH] block: output more error messages if failed to create temporary snapshot
From: Dunrong Huang riegama...@gmail.com
If we failed to create temporary snapshot, the error message did not match
with the error, for example:
$ TMPDIR=/tmp/bad_path qemu-system-x86_64 -enable-kvm debian.qcow2 -snapshot
qemu-system-x86_64: -enable-kvm: could not open disk image
/home/mathslinux/Images/debian.qcow2: No such file or directory
Indeed, the file which cant be created is /tmp/bad_path/vl.xx, not
debian.qcow2. so the error message makes users feel confused.
Signed-off-by: Dunrong Huang riegama...@gmail.com
---
block.c | 2 ++
1 个文件被修改，插入 2 行(+)
diff --git a/block.c b/block.c
index 470bdcc..c9e5140 100644
--- a/block.c
+++ b/block.c
@@ -434,6 +434,8 @@ int get_tmp_filename(char *filename, int size)
}
fd = mkstemp(filename);
if (fd 0 || close(fd)) {
+fprintf(stderr, Could not create temporary snapshot in %s directory:
+%s\n, tmpdir, strerror(errno));
return -errno;
}
return 0;
--
1.7.12
Re: [Qemu-devel] [PATCH v2 2/7] s390/kvm: Add support for machine checks.
On Wed, 5 Sep 2012 09:22:32 +0200
Heiko Carstens heiko.carst...@de.ibm.com wrote:
On Tue, Sep 04, 2012 at 05:13:25PM +0200, Cornelia Huck wrote:
Just some quick comments:
[...]
int kvm_s390_inject_program_int(struct kvm_vcpu *vcpu, u16 code)
{
struct kvm_s390_local_interrupt *li = vcpu-arch.local_int;
@@ -648,6 +747,12 @@ int kvm_s390_inject_vm(struct kvm *kvm,
case KVM_S390_INT_EMERGENCY:
kfree(inti);
return -EINVAL;
+ case KVM_S390_MCHK:
+ VM_EVENT(kvm, 5, inject: machine check parm64:%llx,
+s390int-parm64);
+ inti-type = s390int-type;
+ inti-mchk.mcic = s390int-parm64;
+ break;
The kvm_s390_interrupt struct seems to be inappropriate to pass machine check
data around.
E.g. if you want to inject an uncorrectable storage error, because the host
failed to swap in a page, you must also pass a failing storage address which
doesn't fit into this structure.
Just something you should consider. ;)
Sigh, it seems we might want a KVM_S390_INTERRUPT2 taking a larger and
more complex structure later on...
kvm_s390_interrupt should be fine for our current needs, though,
expecially as machine checks are currently only injected in-kernel.
+static int handle_lpswe(struct kvm_vcpu *vcpu)
+{
+ int base2 = vcpu-arch.sie_block-ipb 28;
+ int disp2 = ((vcpu-arch.sie_block-ipb 0x0fff) 16);
Sooner or later we need helper functions which extract the significant parts
of an instruction.
Maybay something like insn_[type]_get_base2(...) or simply structures like
struct insn_[type], which allow to easily access parts of an instruction.
Agree on helper functions, not sure about the use of structures for
that. Let's see how it looks coded up.
+ u64 addr;
+ u64 new_psw[2];
psw_t?
+
+ addr = disp2;
+ if (base2)
+ addr += vcpu-run-s.regs.gprs[base2];
+
+ if (addr 7) {
+ kvm_s390_inject_program_int(vcpu, PGM_SPECIFICATION);
+ goto out;
+ }
+
+ if (copy_from_guest(vcpu, new_psw, addr, sizeof(*new_psw))) {
I assume that should be sizeof(new_psw). Did that ever work?!
No, because the code was never hit since the code for ICTL was
incorrect... The guest kernel just does a good job at keeping machine
checks open nearly all of the time :)
+ if ((vcpu-arch.sie_block-gpsw.mask 0xb80800fe7fff) ||
+ (((vcpu-arch.sie_block-gpsw.mask 0x00011000) ==
+ 0x1000)
+(vcpu-arch.sie_block-gpsw.addr 0x8000)) ||
+ (!(vcpu-arch.sie_block-gpsw.mask 0x00018000)
+(vcpu-arch.sie_block-gpsw.addr 0xfff0)) ||
+ ((vcpu-arch.sie_block-gpsw.mask 0x00011000) ==
+0x0001)) {
This is not very readable...
It's straight from the PoP's discussion of invalid bits.
Please make use of the PSW defines in ptrace.h and add new ones if needed.
Also please make use of (move) the PSW32 defines in compat.h.
I'll check these.
Re: [Qemu-devel] [RFC 1/6] x86_cpudef_setup: coding style change
On Fri, 17 Aug 2012 14:53:37 -0300
Eduardo Habkost ehabk...@redhat.com wrote:
Make source code lines shorter.
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 96ec9e6..519a104 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1500,20 +1500,23 @@ void x86_cpudef_setup(void)
static const char *model_with_versions[] = { qemu32, qemu64,
athlon };
for (i = 0; i ARRAY_SIZE(builtin_x86_defs); ++i) {
-builtin_x86_defs[i].next = x86_defs;
-builtin_x86_defs[i].flags = 1;
+x86_def_t *def = builtin_x86_defs[i];
+def-next = x86_defs;
+def-flags = 1;
/* Look for specific cpudef models that */
/* have the QEMU version in .model_id */
for (j = 0; j ARRAY_SIZE(model_with_versions); j++) {
-if (strcmp(model_with_versions[j], builtin_x86_defs[i].name)
== 0) {
-pstrcpy(builtin_x86_defs[i].model_id,
sizeof(builtin_x86_defs[i].model_id), QEMU Virtual CPU version );
-pstrcat(builtin_x86_defs[i].model_id,
sizeof(builtin_x86_defs[i].model_id), qemu_get_version());
+if (strcmp(model_with_versions[j], def-name) == 0) {
+pstrcpy(def-model_id, sizeof(def-model_id),
+QEMU Virtual CPU version );
+pstrcat(def-model_id, sizeof(def-model_id),
+qemu_get_version());
break;
}
}
-x86_defs = builtin_x86_defs[i];
+x86_defs = def;
}
#if !defined(CONFIG_USER_ONLY)
qemu_opts_foreach(qemu_find_opts(cpudef), cpudef_register, NULL, 0);
Reviewed-By: Igor Mammedov imamm...@redhat.com
Re: [Qemu-devel] [PATCH] Versatile Express: add modelling of NOR flash
On 5 September 2012 06:16, Stefan Weil s...@weilnetz.de wrote: Am 04.09.2012 19:08, schrieb Francesco Lavra: /* VE_NORFLASH0ALIAS: not modelled */ What about that alias? It's not difficult to add it, too. Just look for memory_region_init_alias in the code to see how it is done (hw/mips_malta.c has an alias region for flash). It's painful because you might also have to add the logic for letting the guest map and unmap the alias (which implies implementing a whole section of the A15 board we don't currently bother with, the SCC registers). I'd need to check the board documentation more carefully to see if we can get away with always mapping that area as the flash alias. (Also we'd need to fix the current problem with the motherboard address map arrays that there's no way to distinguish peripheral not present on this board from peripheral at address 0, since the A9 board doesn't have the flash alias.) More to the point, this is the third attempt at doing this. Previously Liming Wang sent a patch: http://patchwork.ozlabs.org/patch/147905/ and Jagan sent a two-patch set: http://patchwork.ozlabs.org/patch/171812/ http://patchwork.ozlabs.org/patch/171814/ both of which failed in the code review stage. Francesco, can you check that you haven't fallen into any of the same problems they did, please? -- PMM
Re: [Qemu-devel] [RFC 3/6] i386: kvm: bit 10 of CPUID[8000_0001].EDX is reserved
On Fri, 17 Aug 2012 14:53:39 -0300 Eduardo Habkost ehabk...@redhat.com wrote: Bit 10 of CPUID[8000_0001].EDX is not defined as an alias of CPUID[1].EDX[10], so do not duplicate it on kvm_arch_get_supported_cpuid(). Signed-off-by: Eduardo Habkost ehabk...@redhat.com --- target-i386/kvm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 696b14a..bab1ef8 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -163,7 +163,7 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function, * so add missing bits according to the AMD spec: */ cpuid_1_edx = kvm_arch_get_supported_cpuid(s, 1, 0, R_EDX); -ret |= cpuid_1_edx 0x183f7ff; +ret |= cpuid_1_edx 0x183f3ff; break; } break; Reviewed-By: Igor Mammedov imamm...@redhat.com
Re: [Qemu-devel] [RFC 4/6] i386: kvm: use a #define for the set of alias feature bits
On Fri, 17 Aug 2012 14:53:40 -0300 Eduardo Habkost ehabk...@redhat.com wrote: Instea of using a hardcoded hex constant, define CPUID_EXT2_AMD_ALIASES as the set of CPUID[8000_0001].EDX bits that on AMD are the same as the bits of CPUID[1].EDX. Signed-off-by: Eduardo Habkost ehabk...@redhat.com --- target-i386/cpu.h | 12 target-i386/kvm.c | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 7b6340c..ba147ac 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -409,6 +409,7 @@ #define CPUID_EXT_HYPERVISOR (1 31) #define CPUID_EXT2_FPU (1 0) +#define CPUID_EXT2_VME (1 1) #define CPUID_EXT2_DE (1 2) #define CPUID_EXT2_PSE (1 3) #define CPUID_EXT2_TSC (1 4) @@ -436,6 +437,17 @@ #define CPUID_EXT2_3DNOWEXT (1 30) #define CPUID_EXT2_3DNOW (1 31) +/* CPUID[8000_0001].EDX bits that are aliase of CPUID[1].EDX bits on AMD CPUs */ +#define CPUID_EXT2_AMD_ALIASES (CPUID_EXT2_FPU | CPUID_EXT2_VME | \ +CPUID_EXT2_DE | CPUID_EXT2_PSE | \ +CPUID_EXT2_TSC | CPUID_EXT2_MSR | \ +CPUID_EXT2_PAE | CPUID_EXT2_MCE | \ +CPUID_EXT2_CX8 | CPUID_EXT2_APIC | \ +CPUID_EXT2_MTRR | CPUID_EXT2_PGE | \ +CPUID_EXT2_MCA | CPUID_EXT2_CMOV | \ +CPUID_EXT2_PAT | CPUID_EXT2_PSE36 | \ +CPUID_EXT2_MMX | CPUID_EXT2_FXSR) + #define CPUID_EXT3_LAHF_LM (1 0) #define CPUID_EXT3_CMP_LEG (1 1) #define CPUID_EXT3_SVM (1 2) diff --git a/target-i386/kvm.c b/target-i386/kvm.c index bab1ef8..63dac56 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -163,7 +163,7 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function, * so add missing bits according to the AMD spec: */ cpuid_1_edx = kvm_arch_get_supported_cpuid(s, 1, 0, R_EDX); -ret |= cpuid_1_edx 0x183f3ff; +ret |= cpuid_1_edx CPUID_EXT2_AMD_ALIASES; break; } break; Reviewed-By: Igor Mammedov imamm...@redhat.com
[Qemu-devel] [PATCH] arch_init.c: add missing '%' symbols before PRIu64 in debug printfs
'%' symbols were missing in front of PRIu64 macros in DPRINTF() messages in
arch_init.c, this caused compilation warnings when compiled with
DEBUG_ARCH_INIT defined.
Signed-off-by: Igor Mitsyanko i.mitsya...@samsung.com
---
arch_init.c |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 5a1173e..47977de 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -562,7 +562,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
if ((i 63) == 0) {
uint64_t t1 = (qemu_get_clock_ns(rt_clock) - bwidth) / 100;
if (t1 MAX_WAIT) {
-DPRINTF(big wait: PRIu64 milliseconds, %d iterations\n,
+DPRINTF(big wait: % PRIu64 milliseconds, %d iterations\n,
t1, i);
break;
}
@@ -587,7 +587,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
expected_time = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth;
-DPRINTF(ram_save_live: expected( PRIu64 ) = max( PRIu64 )?\n,
+DPRINTF(ram_save_live: expected(% PRIu64 ) = max(% PRIu64 )?\n,
expected_time, migrate_max_downtime());
if (expected_time = migrate_max_downtime()) {
@@ -799,8 +799,8 @@ static int ram_load(QEMUFile *f, void *opaque, int
version_id)
} while (!(flags RAM_SAVE_FLAG_EOS));
done:
-DPRINTF(Completed load of VM with exit code %d seq iteration PRIu64
\n,
-ret, seq_iter);
+DPRINTF(Completed load of VM with exit code %d seq iteration
+% PRIu64 \n, ret, seq_iter);
return ret;
}
--
1.7.5.4
Re: [Qemu-devel] [RFC 2/6] i386: kill cpudef config section support
On Fri, 17 Aug 2012 14:53:38 -0300
Eduardo Habkost ehabk...@redhat.com wrote:
It's nice to have a flexible system to maintain CPU models as data, but
this is holding us from making improvements in the CPU code because it's
not using the common infra-structure, and because the machine-type data
is still inside C code.
Users who want to configure CPU features directly may simply use the
-cpu command-line option (and maybe an equivalent -device option in
the future) to set CPU features.
Have I missed a patch that moves CPU definitions from cpus-x86_64.conf into
the code?
Perhaps they should be added to builtin_x86_defs in this patch.
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 101
++ 1 file changed, 2
insertions(+), 99 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 519a104..71c2ee7 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -237,7 +237,6 @@ typedef struct x86_def_t {
uint32_t xlevel;
char model_id[48];
int vendor_override;
-uint32_t flags;
/* Store the results of Centaur's CPUID instructions */
uint32_t ext4_features;
uint32_t xlevel2;
@@ -1286,7 +1285,7 @@ void x86_cpu_list(FILE *f, fprintf_function
cpu_fprintf) char buf[256];
for (def = x86_defs; def; def = def-next) {
-snprintf(buf, sizeof (buf), def-flags ? [%s]: %s, def-name);
+snprintf(buf, sizeof(buf), %s, def-name);
(*cpu_fprintf)(f, x86 %16s %-48s\n, buf, def-model_id);
}
if (kvm_enabled()) {
@@ -1380,16 +1379,6 @@ int cpu_x86_register(X86CPU *cpu, const char
*cpu_model) }
#if !defined(CONFIG_USER_ONLY)
-/* copy vendor id string to 32 bit register, nul pad as needed
- */
-static void cpyid(const char *s, uint32_t *id)
-{
-char *d = (char *)id;
-char i;
-
-for (i = sizeof (*id); i--; )
-*d++ = *s ? *s++ : '\0';
-}
/* interpret radix and convert from string to arbitrary scalar,
* otherwise flag failure
@@ -1403,87 +1392,6 @@ static void cpyid(const char *s, uint32_t *id)
*str !*pend ? (*pval = ul) : (*perr = 1);\
}
-/* map cpuid options to feature bits, otherwise return failure
- * (option tags in *str are delimited by whitespace)
- */
-static void setfeatures(uint32_t *pval, const char *str,
-const char **featureset, int *perr)
-{
-const char *p, *q;
-
-for (q = p = str; *p || *q; q = p) {
-while (iswhite(*p))
-q = ++p;
-while (*p !iswhite(*p))
-++p;
-if (!*q !*p)
-return;
-if (!lookup_feature(pval, q, p, featureset)) {
-fprintf(stderr, error: feature \%.*s\ not available in
set\n,
-(int)(p - q), q);
-*perr = 1;
-return;
-}
-}
-}
-
-/* map config file options to x86_def_t form
- */
-static int cpudef_setfield(const char *name, const char *str, void *opaque)
-{
-x86_def_t *def = opaque;
-int err = 0;
-
-if (!strcmp(name, name)) {
-g_free((void *)def-name);
-def-name = g_strdup(str);
-} else if (!strcmp(name, model_id)) {
-strncpy(def-model_id, str, sizeof (def-model_id));
-} else if (!strcmp(name, level)) {
-setscalar(def-level, str, err)
-} else if (!strcmp(name, vendor)) {
-cpyid(str[0], def-vendor1);
-cpyid(str[4], def-vendor2);
-cpyid(str[8], def-vendor3);
-} else if (!strcmp(name, family)) {
-setscalar(def-family, str, err)
-} else if (!strcmp(name, model)) {
-setscalar(def-model, str, err)
-} else if (!strcmp(name, stepping)) {
-setscalar(def-stepping, str, err)
-} else if (!strcmp(name, feature_edx)) {
-setfeatures(def-features, str, feature_name, err);
-} else if (!strcmp(name, feature_ecx)) {
-setfeatures(def-ext_features, str, ext_feature_name, err);
-} else if (!strcmp(name, extfeature_edx)) {
-setfeatures(def-ext2_features, str, ext2_feature_name, err);
-} else if (!strcmp(name, extfeature_ecx)) {
-setfeatures(def-ext3_features, str, ext3_feature_name, err);
-} else if (!strcmp(name, xlevel)) {
-setscalar(def-xlevel, str, err)
-} else {
-fprintf(stderr, error: unknown option [%s = %s]\n, name, str);
-return (1);
-}
-if (err) {
-fprintf(stderr, error: bad option value [%s = %s]\n, name, str);
-return (1);
-}
-return (0);
-}
-
-/* register config file entry as x86_def_t
- */
-static int cpudef_register(QemuOpts *opts, void *opaque)
-{
-x86_def_t *def = g_malloc0(sizeof (x86_def_t));
-
-qemu_opt_foreach(opts, cpudef_setfield, def, 1);
-def-next = x86_defs;
-x86_defs = def;
-return (0);
-}
-
void cpu_clear_apic_feature(CPUX86State *env)
{
env-cpuid_features =
[Qemu-devel] [PATCH] fix entry pointer for ELF kernels loaded with -kernel option
This patch fixes a bug in qemu which prevents Multiboot ELF kernels from being loaded with the -kernel option. Find a full description of the problem here https://bugs.launchpad.net/qemu/+bug/1044727 . --- hw/elf_ops.h | 10 ++ 1 files changed, 10 insertions(+), 0 deletions(-) diff --git a/hw/elf_ops.h b/hw/elf_ops.h index fa65ce2..aeddd11 100644 --- a/hw/elf_ops.h +++ b/hw/elf_ops.h @@ -269,6 +269,16 @@ static int glue(load_elf, SZ)(const char *name, int fd, addr = ph-p_paddr; } +/* the entry pointer in the ELF header is a virtual + * address, if the text segments paddr and vaddr differ + * we need to adjust the entry */ +if (pentry !translate_fn +ph-p_vaddr != ph-p_paddr +ehdr.e_entry = ph-p_vaddr +ehdr.e_entry ph-p_vaddr + ph-p_filesz +ph-p_flags PF_X) +*pentry = ehdr.e_entry - ph-p_vaddr + ph-p_paddr; + snprintf(label, sizeof(label), phdr #%d: %s, i, name); rom_add_blob_fixed(label, data, mem_size, addr); -- 1.7.8.6
Re: [Qemu-devel] [PATCH 0/3] fdc: Fix FD_SR0_SEEK flag
On 09/04/2012 11:46 AM, Kevin Wolf wrote: Pavel, does this make sense to you? It seems to fix the NT 3.1 floppy driver. Kevin Wolf (3): fdc: Remove status0 parameter from fdctrl_set_fifo() fdc: Fix false FD_SR0_SEEK fdc-test: Check READ ID hw/fdc.c | 38 ++ tests/fdc-test.c | 68 +- 2 files changed, 85 insertions(+), 21 deletions(-) Hi Kevin, yes, it make sense to me. It's definitely better now.
Re: [Qemu-devel] [PATCH 0/2 v3] Fix static linking for cURL and SDL
On Sun, Sep 02, 2012 at 03:09:44PM +0200, Yann E. MORIN wrote:
Hello All!
Currently, configure checks for cURL and SDL with either pkg-config (the
default), or with {curl,sdl}-config (as a fallback).
But pkg-config and {curl,sdl}-config do not have the same set of options:
- to check for shared libs, both use the option: --libs
- to check for static libs:
- pkg-config uses : --static --libs
- {curl,sdl}-config use: --static-libs
To add to the complexity, pkg-config is called through the querry_pkg_config
wrapper, that already passes --static when static linking is required, but
there is no such wrapper for {curl,sdl}-config, so we miss the occasion to
pass --static-libs.
To fix this:
- introduce a new variable QEMU_XXX_CONFIG_LIBS_FLAGS that mirrors the
behavior of QEMU_PKG_CONFIG_FLAGS; this variable can be used by all
xxx-config scripts (eg. curl-config, but later sdl-config too).
Default it to '--libs', which is for shared linking.
- properly use either --libs for pkg-config (--static is already taken
care of in the wrapper), or $QEMU_XXX_CONFIG_LIBS_FLAGS for
{curl,sdl}-config.
Changes since v2:
- remove trailing reference to cURL in the SDL patch (Stefan Hajnoczi)
- sent to qemu-devel and cc qemu-trivial (Peter Maydell, Stefan)
- fix type in the name of the new variable
Changes since v1:
- drop the spice fix, it is not needed (bad env locally)
- drop the added --static to calls to pkg-config, as it's already in the
wrapper (Stefan Hajnoczi)
Any more discussion around this?
I'm happy with the patches.
Stefan
Re: [Qemu-devel] [Qemu-trivial] [PATCH] sparc-dis: Remove redundant NULL check
On Mon, Sep 03, 2012 at 10:23:23PM +0200, Stefan Weil wrote: Report from smatch: sparc-dis.c:2664 build_hash_table(14) info: redundant null check on hash_buf calling free() Signed-off-by: Stefan Weil s...@weilnetz.de --- Coding style was not fixed. - sw sparc-dis.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) Thanks, applied to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
Re: [Qemu-devel] [PATCH] fix entry pointer for ELF kernels loaded with -kernel option
2012/9/5 Henning Schild henn...@hennsch.de:
This patch fixes a bug in qemu which prevents Multiboot ELF kernels from
being loaded with the -kernel option. Find a full description of the problem
here https://bugs.launchpad.net/qemu/+bug/1044727 .
---
hw/elf_ops.h | 10 ++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/hw/elf_ops.h b/hw/elf_ops.h
index fa65ce2..aeddd11 100644
--- a/hw/elf_ops.h
+++ b/hw/elf_ops.h
@@ -269,6 +269,16 @@ static int glue(load_elf, SZ)(const char *name, int fd,
addr = ph-p_paddr;
}
+/* the entry pointer in the ELF header is a virtual
+ * address, if the text segments paddr and vaddr differ
+ * we need to adjust the entry */
+if (pentry !translate_fn
+ph-p_vaddr != ph-p_paddr
+ehdr.e_entry = ph-p_vaddr
+ehdr.e_entry ph-p_vaddr + ph-p_filesz
+ph-p_flags PF_X)
According to the code style, braces {} are needed.
+*pentry = ehdr.e_entry - ph-p_vaddr + ph-p_paddr;
+
snprintf(label, sizeof(label), phdr #%d: %s, i, name);
rom_add_blob_fixed(label, data, mem_size, addr);
--
1.7.8.6
--
Best Regards,
Dunrong Huang
Re: [Qemu-devel] [PATCH] kvm: Fix warning from static code analysis
On Mon, Sep 03, 2012 at 10:40:40PM +0200, Stefan Weil wrote: Report from smatch: kvm-all.c:1373 kvm_init(135) warn: variable dereferenced before check 's' (see line 1360) 's' cannot by NULL (it was alloced using g_malloc0), so there is no need to check it here. Signed-off-by: Stefan Weil s...@weilnetz.de --- kvm-all.c | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-) Thanks, applied to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
Re: [Qemu-devel] [RFC 2/6] i386: kill cpudef config section support
On Wed, 5 Sep 2012 11:09:16 +0200
Igor Mammedov imamm...@redhat.com wrote:
On Fri, 17 Aug 2012 14:53:38 -0300
Eduardo Habkost ehabk...@redhat.com wrote:
It's nice to have a flexible system to maintain CPU models as data, but
this is holding us from making improvements in the CPU code because it's
not using the common infra-structure, and because the machine-type data
is still inside C code.
Users who want to configure CPU features directly may simply use the
-cpu command-line option (and maybe an equivalent -device option in
the future) to set CPU features.
Have I missed a patch that moves CPU definitions from cpus-x86_64.conf into
the code?
Perhaps they should be added to builtin_x86_defs in this patch.
Ah, found it
http://lists.gnu.org/archive/html/qemu-devel/2012-08/msg00223.html
It seems missed 1.2 and this series just don't mention that it depends on
above mentioned series.
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 101
++ 1 file changed, 2
insertions(+), 99 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 519a104..71c2ee7 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -237,7 +237,6 @@ typedef struct x86_def_t {
uint32_t xlevel;
char model_id[48];
int vendor_override;
-uint32_t flags;
/* Store the results of Centaur's CPUID instructions */
uint32_t ext4_features;
uint32_t xlevel2;
@@ -1286,7 +1285,7 @@ void x86_cpu_list(FILE *f, fprintf_function
cpu_fprintf) char buf[256];
for (def = x86_defs; def; def = def-next) {
-snprintf(buf, sizeof (buf), def-flags ? [%s]: %s,
def-name);
+snprintf(buf, sizeof(buf), %s, def-name);
(*cpu_fprintf)(f, x86 %16s %-48s\n, buf, def-model_id);
}
if (kvm_enabled()) {
@@ -1380,16 +1379,6 @@ int cpu_x86_register(X86CPU *cpu, const char
*cpu_model) }
#if !defined(CONFIG_USER_ONLY)
-/* copy vendor id string to 32 bit register, nul pad as needed
- */
-static void cpyid(const char *s, uint32_t *id)
-{
-char *d = (char *)id;
-char i;
-
-for (i = sizeof (*id); i--; )
-*d++ = *s ? *s++ : '\0';
-}
/* interpret radix and convert from string to arbitrary scalar,
* otherwise flag failure
@@ -1403,87 +1392,6 @@ static void cpyid(const char *s, uint32_t *id)
*str !*pend ? (*pval = ul) : (*perr = 1);\
}
-/* map cpuid options to feature bits, otherwise return failure
- * (option tags in *str are delimited by whitespace)
- */
-static void setfeatures(uint32_t *pval, const char *str,
-const char **featureset, int *perr)
-{
-const char *p, *q;
-
-for (q = p = str; *p || *q; q = p) {
-while (iswhite(*p))
-q = ++p;
-while (*p !iswhite(*p))
-++p;
-if (!*q !*p)
-return;
-if (!lookup_feature(pval, q, p, featureset)) {
-fprintf(stderr, error: feature \%.*s\ not available in
set\n,
-(int)(p - q), q);
-*perr = 1;
-return;
-}
-}
-}
-
-/* map config file options to x86_def_t form
- */
-static int cpudef_setfield(const char *name, const char *str, void
*opaque) -{
-x86_def_t *def = opaque;
-int err = 0;
-
-if (!strcmp(name, name)) {
-g_free((void *)def-name);
-def-name = g_strdup(str);
-} else if (!strcmp(name, model_id)) {
-strncpy(def-model_id, str, sizeof (def-model_id));
-} else if (!strcmp(name, level)) {
-setscalar(def-level, str, err)
-} else if (!strcmp(name, vendor)) {
-cpyid(str[0], def-vendor1);
-cpyid(str[4], def-vendor2);
-cpyid(str[8], def-vendor3);
-} else if (!strcmp(name, family)) {
-setscalar(def-family, str, err)
-} else if (!strcmp(name, model)) {
-setscalar(def-model, str, err)
-} else if (!strcmp(name, stepping)) {
-setscalar(def-stepping, str, err)
-} else if (!strcmp(name, feature_edx)) {
-setfeatures(def-features, str, feature_name, err);
-} else if (!strcmp(name, feature_ecx)) {
-setfeatures(def-ext_features, str, ext_feature_name, err);
-} else if (!strcmp(name, extfeature_edx)) {
-setfeatures(def-ext2_features, str, ext2_feature_name, err);
-} else if (!strcmp(name, extfeature_ecx)) {
-setfeatures(def-ext3_features, str, ext3_feature_name, err);
-} else if (!strcmp(name, xlevel)) {
-setscalar(def-xlevel, str, err)
-} else {
-fprintf(stderr, error: unknown option [%s = %s]\n, name, str);
-return (1);
-}
-if (err) {
-fprintf(stderr, error: bad option value [%s = %s]\n, name,
str);
-return (1);
-}
-return (0);
Re: [Qemu-devel] [PATCH V7 2/2] qemu-img: Add json output option to the info command.
Am 05.09.2012 11:25, schrieb Benoît Canet:
This option --output=[human|json] make qemu-img info output on
human or JSON representation at the choice of the user.
example:
{
snapshots: [
{
vm-clock-nsec: 637102488,
name: vm-20120821145509,
date-sec: 1345553709,
date-nsec: 220289000,
vm-clock-sec: 20,
id: 1,
vm-state-size: 96522745
},
{
vm-clock-nsec: 28210866,
name: vm-20120821154059,
date-sec: 1345556459,
date-nsec: 171392000,
vm-clock-sec: 46,
id: 2,
vm-state-size: 101208714
}
],
virtual-size: 1073741824,
filename: snap.qcow2,
cluster-size: 65536,
format: qcow2,
actual-size: 985587712,
dirty-flag: false
}
Signed-off-by: Benoit Canet ben...@irqsave.net
Looks much better, but I still have a few comments:
+bdrv_get_backing_filename(bs, backing_filename,
sizeof(backing_filename));
+if (backing_filename[0] != '\0') {
+info-backing_filename = g_strdup(backing_filename);
+info-has_backing_filename = true;
+bdrv_get_full_backing_filename(bs, backing_filename2,
+ sizeof(backing_filename2));
+
+if (strcmp(backing_filename, backing_filename2) != 0) {
+info-full_backing_filename =
+g_strdup(backing_filename2);
+ info-has_full_backing_filename = true;
+}
+
+info-backing_filename_format = bs-backing_format;
+info-has_backing_filename_format = true;
I believe we should check bs-backing_format[0] first. If it's empty,
don't include the format.
+}
+}
+
+static void dump_human_image_info(ImageInfo *info)
+{
+char size_buf[128], dsize_buf[128];
+if (!info-has_actual_size) {
+snprintf(dsize_buf, sizeof(dsize_buf), unavailable);
+} else {
+get_human_readable_size(dsize_buf, sizeof(dsize_buf),
+info-actual_size);
+}
+get_human_readable_size(size_buf, sizeof(size_buf), info-virtual_size);
+printf(image: %s\n
+ file format: %s\n
+ virtual size: %s (% PRId64 bytes)\n
+ disk size: %s\n,
+ info-filename, info-format, size_buf,
+ info-virtual_size,
+ dsize_buf);
+
+if (info-has_encrypted info-encrypted) {
+printf(encrypted: yes\n);
+}
+
+if (info-has_cluster_size) {
+printf(cluster_size: % PRId64 \n, info-cluster_size);
+}
+
+if (info-has_dirty_flag info-dirty_flag) {
+printf(cleanly shut down: no\n);
+}
+
+if (info-has_backing_filename) {
+printf(backing file: %s, info-backing_filename);
+if (info-has_full_backing_filename) {
+printf( (actual path: %s), info-full_backing_filename);
+}
Maybe add the backing file format here if it's available?
@@ -1135,48 +1291,36 @@ static int img_info(int argc, char **argv)
}
filename = argv[optind++];
+if (output !strcmp(output, json)) {
+output_format = OFORMAT_JSON;
+} else if (output !strcmp(output, human)) {
+output_format = OFORMAT_HUMAN;
+} else if (output) {
+error_report(--output must be used with human or json as
argument.);
+return 1;
+}
+
+info = g_new0(ImageInfo, 1);
bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING);
if (!bs) {
+g_free(info);
return 1;
}
If you move thee g_new0 below, you don't have to free here.
diff --git a/qemu-img.texi b/qemu-img.texi
index 6b42e35..75a7c8b 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -129,12 +129,13 @@ created as a copy on write image of the specified base
image; the
@var{backing_file} should have the same content as the input's base image,
however the path, image format, etc may differ.
-@item info [-f @var{fmt}] @var{filename}
+@item info [-f @var{fmt}] [--output=@var{ofmt}] @var{filename}
Give information about the disk image @var{filename}. Use it in
particular to know the size reserved on disk which can be different
from the displayed size. If VM snapshots are stored in the disk image,
-they are displayed too.
+they are displayed too. The command can output in the format @var{ofmt}
+which is either human or json.
...@code{human} or @code{json} is better, I think.
Kevin
[Qemu-devel] [PATCH 2/4] spice: split qemu_spice_create_update
Creating one function which creates a single update for a given
rectangle. And one (for now) pretty simple wrapper around it to
queue up screen updates for the dirty region.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
ui/spice-display.c | 29 +
1 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 59c5fd7..bc2f207 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -164,7 +164,8 @@ int qemu_spice_display_is_running(SimpleSpiceDisplay *ssd)
#endif
}
-static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
+static void qemu_spice_create_one_update(SimpleSpiceDisplay *ssd,
+ QXLRect *rect)
{
SimpleSpiceUpdate *update;
QXLDrawable *drawable;
@@ -174,21 +175,17 @@ static void qemu_spice_create_update(SimpleSpiceDisplay
*ssd)
int by, bw, bh;
struct timespec time_space;
-if (qemu_spice_rect_is_empty(ssd-dirty)) {
-return;
-};
-
trace_qemu_spice_create_update(
- ssd-dirty.left, ssd-dirty.right,
- ssd-dirty.top, ssd-dirty.bottom);
+ rect-left, rect-right,
+ rect-top, rect-bottom);
update = g_malloc0(sizeof(*update));
drawable = update-drawable;
image= update-image;
cmd = update-ext.cmd;
-bw = ssd-dirty.right - ssd-dirty.left;
-bh = ssd-dirty.bottom - ssd-dirty.top;
+bw = rect-right - rect-left;
+bh = rect-bottom - rect-top;
update-bitmap = g_malloc(bw * bh * 4);
drawable-bbox= ssd-dirty;
@@ -226,8 +223,8 @@ static void qemu_spice_create_update(SimpleSpiceDisplay
*ssd)
}
src = ds_get_data(ssd-ds) +
-ssd-dirty.top * ds_get_linesize(ssd-ds) +
-ssd-dirty.left * ds_get_bytes_per_pixel(ssd-ds);
+rect-top * ds_get_linesize(ssd-ds) +
+rect-left * ds_get_bytes_per_pixel(ssd-ds);
dst = update-bitmap;
for (by = 0; by bh; by++) {
qemu_pf_conv_run(ssd-conv, dst, src, bw);
@@ -238,10 +235,18 @@ static void qemu_spice_create_update(SimpleSpiceDisplay
*ssd)
cmd-type = QXL_CMD_DRAW;
cmd-data = (uintptr_t)drawable;
-memset(ssd-dirty, 0, sizeof(ssd-dirty));
QTAILQ_INSERT_TAIL(ssd-updates, update, next);
}
+static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
+{
+if (qemu_spice_rect_is_empty(ssd-dirty)) {
+return;
+};
+qemu_spice_create_one_update(ssd, ssd-dirty);
+memset(ssd-dirty, 0, sizeof(ssd-dirty));
+}
+
/*
* Called from spice server thread context (via interface_release_ressource)
* We do *not* hold the global qemu mutex here, so extra care is needed
--
1.7.1
[Qemu-devel] [PATCH 3/4] spice: add screen mirror
Create a screen mirror, keep there a copy of the most recent update
passed on to spice-server.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
ui/spice-display.c | 32 ++--
ui/spice-display.h |1 +
2 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/ui/spice-display.c b/ui/spice-display.c
index bc2f207..6f87099 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -171,8 +171,8 @@ static void qemu_spice_create_one_update(SimpleSpiceDisplay
*ssd,
QXLDrawable *drawable;
QXLImage *image;
QXLCommand *cmd;
-uint8_t *src, *dst;
-int by, bw, bh;
+uint8_t *src, *mirror, *dst;
+int by, bw, bh, offset, bytes;
struct timespec time_space;
trace_qemu_spice_create_update(
@@ -216,19 +216,18 @@ static void
qemu_spice_create_one_update(SimpleSpiceDisplay *ssd,
image-bitmap.palette = 0;
image-bitmap.format = SPICE_BITMAP_FMT_32BIT;
-if (ssd-conv == NULL) {
-PixelFormat dst = qemu_default_pixelformat(32);
-ssd-conv = qemu_pf_conv_get(dst, ssd-ds-surface-pf);
-assert(ssd-conv);
-}
-
-src = ds_get_data(ssd-ds) +
+offset =
rect-top * ds_get_linesize(ssd-ds) +
rect-left * ds_get_bytes_per_pixel(ssd-ds);
+bytes = ds_get_bytes_per_pixel(ssd-ds) * bw;
+src = ds_get_data(ssd-ds) + offset;
+mirror = ssd-ds_mirror + offset;
dst = update-bitmap;
for (by = 0; by bh; by++) {
-qemu_pf_conv_run(ssd-conv, dst, src, bw);
+memcpy(mirror, src, bytes);
+qemu_pf_conv_run(ssd-conv, dst, mirror, bw);
src += ds_get_linesize(ssd-ds);
+mirror += ds_get_linesize(ssd-ds);
dst += image-bitmap.stride;
}
@@ -243,6 +242,17 @@ static void qemu_spice_create_update(SimpleSpiceDisplay
*ssd)
if (qemu_spice_rect_is_empty(ssd-dirty)) {
return;
};
+
+if (ssd-conv == NULL) {
+PixelFormat dst = qemu_default_pixelformat(32);
+ssd-conv = qemu_pf_conv_get(dst, ssd-ds-surface-pf);
+assert(ssd-conv);
+}
+if (ssd-ds_mirror == NULL) {
+int size = ds_get_height(ssd-ds) * ds_get_linesize(ssd-ds);
+ssd-ds_mirror = g_malloc0(size);
+}
+
qemu_spice_create_one_update(ssd, ssd-dirty);
memset(ssd-dirty, 0, sizeof(ssd-dirty));
}
@@ -358,6 +368,8 @@ void qemu_spice_display_resize(SimpleSpiceDisplay *ssd)
memset(ssd-dirty, 0, sizeof(ssd-dirty));
qemu_pf_conv_put(ssd-conv);
ssd-conv = NULL;
+g_free(ssd-ds_mirror);
+ssd-ds_mirror = NULL;
qemu_mutex_lock(ssd-lock);
while ((update = QTAILQ_FIRST(ssd-updates)) != NULL) {
diff --git a/ui/spice-display.h b/ui/spice-display.h
index 3fcb6fe..dea41c1 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -72,6 +72,7 @@ typedef struct SimpleSpiceUpdate SimpleSpiceUpdate;
struct SimpleSpiceDisplay {
DisplayState *ds;
+uint8_t *ds_mirror;
void *buf;
int bufsize;
QXLWorker *worker;
--
1.7.1
Re: [Qemu-devel] [Qemu-trivial] [PATCH] block: output more error messages if failed to create temporary snapshot
On Wed, Sep 05, 2012 at 04:26:14PM +0800, riegama...@gmail.com wrote:
From: Dunrong Huang riegama...@gmail.com
If we failed to create temporary snapshot, the error message did not match
with the error, for example:
$ TMPDIR=/tmp/bad_path qemu-system-x86_64 -enable-kvm debian.qcow2 -snapshot
qemu-system-x86_64: -enable-kvm: could not open disk image
/home/mathslinux/Images/debian.qcow2: No such file or directory
Indeed, the file which cant be created is /tmp/bad_path/vl.xx, not
debian.qcow2. so the error message makes users feel confused.
Signed-off-by: Dunrong Huang riegama...@gmail.com
---
block.c | 2 ++
1 个文件被修改，插入 2 行(+)
diff --git a/block.c b/block.c
index 470bdcc..c9e5140 100644
--- a/block.c
+++ b/block.c
@@ -434,6 +434,8 @@ int get_tmp_filename(char *filename, int size)
}
fd = mkstemp(filename);
if (fd 0 || close(fd)) {
+fprintf(stderr, Could not create temporary snapshot in %s
directory:
+%s\n, tmpdir, strerror(errno));
The error message is fine for fd 0 but not for close(0) != 0. Also,
close(2) is allowed to change errno (even on success) so this is not
portable and could clobber the errno value.
Please split into:
if (fd 0) {
...
}
if (close(fd) != 0) {
...
}
Stefan
[Qemu-devel] [PATCH 4/4] spice: send updates only for changed screen content
when creating screen updates go compare the current guest screen
against the mirror (which holds the most recent update sent), then
only create updates for the screen areas which did actually change.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
ui/spice-display.c | 55
1 files changed, 55 insertions(+), 0 deletions(-)
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 6f87099..ea87d82 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -239,6 +239,13 @@ static void
qemu_spice_create_one_update(SimpleSpiceDisplay *ssd,
static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
{
+static const int blksize = 42;
+int blocks = (ds_get_width(ssd-ds) + blksize - 1) / blksize;
+int dirty_top[blocks];
+int y, yoff, x, xoff, blk, bw;
+int bpp = ds_get_bytes_per_pixel(ssd-ds);
+uint8_t *guest, *mirror;
+
if (qemu_spice_rect_is_empty(ssd-dirty)) {
return;
};
@@ -253,6 +260,54 @@ static void qemu_spice_create_update(SimpleSpiceDisplay
*ssd)
ssd-ds_mirror = g_malloc0(size);
}
+for (blk = 0; blk blocks; blk++) {
+dirty_top[blk] = -1;
+}
+
+guest = ds_get_data(ssd-ds);
+mirror = ssd-ds_mirror;
+for (y = ssd-dirty.top; y ssd-dirty.bottom; y++) {
+yoff = y * ds_get_linesize(ssd-ds);
+for (x = ssd-dirty.left; x ssd-dirty.right; x += blksize) {
+xoff = x * bpp;
+blk = x / blksize;
+bw = MIN(blksize, ssd-dirty.right - x);
+if (memcmp(guest + yoff + xoff,
+ mirror + yoff + xoff,
+ bw * bpp) == 0) {
+if (dirty_top[blk] != -1) {
+QXLRect update = {
+.top= dirty_top[blk],
+.bottom = y,
+.left = x,
+.right = x + bw,
+};
+qemu_spice_create_one_update(ssd, update);
+dirty_top[blk] = -1;
+}
+} else {
+if (dirty_top[blk] == -1) {
+dirty_top[blk] = y;
+}
+}
+}
+}
+
+for (x = ssd-dirty.left; x ssd-dirty.right; x += blksize) {
+blk = x / blksize;
+bw = MIN(blksize, ssd-dirty.right - x);
+if (dirty_top[blk] != -1) {
+QXLRect update = {
+.top= dirty_top[blk],
+.bottom = ssd-dirty.bottom,
+.left = x,
+.right = x + bw,
+};
+qemu_spice_create_one_update(ssd, update);
+dirty_top[blk] = -1;
+}
+}
+
qemu_spice_create_one_update(ssd, ssd-dirty);
memset(ssd-dirty, 0, sizeof(ssd-dirty));
}
--
1.7.1
Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem
On 09/05/2012 11:19 AM, liu ping fan wrote: [...] We could externalize the refcounting and push it into device code. This means: memory_region_init_io(s-mem, dev) ... object_ref(dev) memory_region_add_subregion(..., dev-mr) ... memory_region_del_subregion(..., dev-mr) // implied flush object_unref(dev) I think object_ref(dev) just another style to push MemoryRegionOps::object() to device level. And I think we can bypass it. The caller (unplug, pci-reconfig ) of memory_region_del_subregion() ensure the @dev is valid. If the implied flush is implemented in synchronize, _del_subregion() will guarantee no reader for @dev-mr and @dev exist any longer. The above code has a deadlock. memory_region_del_subregion() may be called under the device lock (since it may be the result of mmio to the device), and if the flush uses synchronized_rcu(), it will wait forever for the read-side critical section to complete. But if _del_subregion() just wait for mr-X quiescent period, while calling in mr-Y's read side critical section, then we can avoid deadlock. I saw in pci-mapping, we delete mr-X in mr-Y read side. So I think we can save both object_ref/unref(dev) for memory system. The only problem is that whether we can implement it as synchronous or not, is it possible that we can launch a _del_subregion(mr-X) in mr-X's dispatcher? Yes. Real cases exist. Oh, I find the sample code, then, the deadlock is unavoidable in this method. What alternatives remain? I think a way out may be async+refcnt If we consider the relationship of MemoryRegionImpl and device as the one between file and file-private_data in Linux. Then the creation of impl will object_ref(dev) and when impl-ref=0, it will object_unref(dev) But this is an async model, for those client which need to know the end of flush, we can adopt callback just like call_rcu(). During this thread, it seems that no matter we adopt refcnt on MemoryRegionImpl or not, protecting MemoryRegion::opaque during dispatch is still required. Right. So MemoryRegionImpl is pointless. My current preference is MemoryRegionOps::ref(MemoryRegion *mr) (and a corresponding unref), which has the following requirements: - if the refcount is nonzero, MemoryRegion::opaque is safe to use - if the refcount is nonzero, the MemoryRegion itself is stable. Later we can change other callbacks to also use mr instead of opaque. Usually the callback will be able to derive the device object from the region, only special cases will require memory_region_opaque(MemoryRegion *). We can even drop the opaque from memory_region_init_io() and replace it with memory_region_set_opaque(), to be used in those special cases. It is challenging to substitute memory_region_init_io() 's 3rd parameter from void* to Object*, owning to the conflict that life-cycle management need the host of opaque, while Object and mr need 1:1 map. So I think, I will move forward on the way based on MemoryRegionOps::object(). Right? As outlined above, I now prefer MemoryRegionOps::ref/unref. Advantages compared to MemoryRegionOps::object(): - decoupled from the QOM framework Disadvantages: - more boilerplate code in converted devices Since we are likely to convert few devices to lockless dispatch, I believe the tradeoff is justified. But let's hear Jan and the others. -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH v2] block: output more error messages if failed to create temporary snapshot
From: Dunrong Huang riegama...@gmail.com
If we failed to create temporary snapshot, the error message did not match
with the error, for example:
$ TMPDIR=/tmp/bad_path qemu-system-x86_64 -enable-kvm debian.qcow2 -snapshot
qemu-system-x86_64: -enable-kvm: could not open disk image
/home/mathslinux/Images/debian.qcow2: No such file or directory
Indeed, the file which cant be created is /tmp/bad_path/vl.xx, not
debian.qcow2. so the error message makes users feel confused.
Signed-off-by: Dunrong Huang riegama...@gmail.com
---
v1 - v2:
Output error message only if fd 0
block.c | 7 ++-
1 个文件被修改，插入 6 行(+)，删除 1 行(-)
diff --git a/block.c b/block.c
index 470bdcc..2dd0c5b 100644
--- a/block.c
+++ b/block.c
@@ -433,7 +433,12 @@ int get_tmp_filename(char *filename, int size)
return -EOVERFLOW;
}
fd = mkstemp(filename);
-if (fd 0 || close(fd)) {
+if (fd 0) {
+fprintf(stderr, Could not create temporary snapshot in %s directory:
+%s\n, tmpdir, strerror(errno));
+return -errno;
+}
+if (close(fd) != 0) {
return -errno;
}
return 0;
--
1.7.12
Re: [Qemu-devel] [PATCH v6 2/2] block: Support GlusterFS as a QEMU block backend
On Wed, Sep 05, 2012 at 01:11:06PM +0530, Bharata B Rao wrote:
On Thu, Aug 09, 2012 at 06:32:16PM +0530, Bharata B Rao wrote:
+static void qemu_gluster_complete_aio(GlusterAIOCB *acb)
+{
+int ret;
+
+if (acb-canceled) {
+qemu_aio_release(acb);
+return;
+}
+
+if (acb-ret == acb-size) {
+ret = 0; /* Success */
+} else if (acb-ret 0) {
+ret = acb-ret; /* Read/Write failed */
+} else {
+ret = -EIO; /* Partial read/write - fail it */
+}
+acb-common.cb(acb-common.opaque, ret);
The .cb() here is bdrv_co_io_em_complete(). It does qemu_coroutine_enter(),
handles the return value and comes back here.
But if the bdrv_read or bdrv_write or bdrv_flush was called from a
coroutine context (as against they themselves creating a new coroutine),
the above .cb() call above doesn't return to this point. Hence I won't
be able to release the acb and decrement the qemu_aio_count.
What could be the issue here ? In general, how do I ensure that my
aio calls get completed correctly in such scenarios where bdrv_read etc
are called from coroutine context rather than from main thread context ?
One way to handle this is not to do completion from gluster thread but
instead schedule a BH that does the completion. In fact I had this approach
in the earlier versions, but resorted to directly calling completion from
gluster thread as I didn't see the value of using a BH for completion.
But I guess its indeed needed to support such scenarios (qcow2 image creation
on gluster backend).
Regards,
Bharata.
Re: [Qemu-devel] [PATCH] arch_init.c: add missing '%' symbols before PRIu64 in debug printfs
On Wed, Sep 05, 2012 at 01:04:56PM +0400, Igor Mitsyanko wrote: '%' symbols were missing in front of PRIu64 macros in DPRINTF() messages in arch_init.c, this caused compilation warnings when compiled with DEBUG_ARCH_INIT defined. Signed-off-by: Igor Mitsyanko i.mitsya...@samsung.com --- arch_init.c |8 1 files changed, 4 insertions(+), 4 deletions(-) Thanks, applied to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
Re: [Qemu-devel] [PATCH v6 2/2] block: Support GlusterFS as a QEMU block backend
Am 05.09.2012 09:41, schrieb Bharata B Rao:
On Thu, Aug 09, 2012 at 06:32:16PM +0530, Bharata B Rao wrote:
+static void qemu_gluster_complete_aio(GlusterAIOCB *acb)
+{
+int ret;
+
+if (acb-canceled) {
+qemu_aio_release(acb);
+return;
+}
+
+if (acb-ret == acb-size) {
+ret = 0; /* Success */
+} else if (acb-ret 0) {
+ret = acb-ret; /* Read/Write failed */
+} else {
+ret = -EIO; /* Partial read/write - fail it */
+}
+acb-common.cb(acb-common.opaque, ret);
The .cb() here is bdrv_co_io_em_complete(). It does qemu_coroutine_enter(),
handles the return value and comes back here.
Right.
.cb is set by qemu_gluster_aio_rw/flush(), and the only way these can be
called is through bdrv_co_io_em() and bdrv_co_flush(), which both set
bdrv_co_io_em_complete as the callback.
But if the bdrv_read or bdrv_write or bdrv_flush was called from a
coroutine context (as against they themselves creating a new coroutine),
the above .cb() call above doesn't return to this point.
Why?
A coroutine that yields before it's completed must be reentered, no
matter whether it's been created for a single request or if it already
existed.
Conversely, a coroutine that you enter, always yields at some point and
then you return from the qemu_coroutine_enter() and get back to this
line of code.
If you never come back to this point, there's a bug somewhere.
Hence I won't
be able to release the acb and decrement the qemu_aio_count.
What could be the issue here ? In general, how do I ensure that my
aio calls get completed correctly in such scenarios where bdrv_read etc
are called from coroutine context rather than from main thread context ?
Creating qcow2 image would lead to this scenario where
-bdrv_create (=qcow2_create) will create a coroutine and subsequently
read and write are called within qcow2_create in coroutine context itself.
Can you describe in more detail what code paths it's taking and at which
point you're thinking it's wrong?
Kevin
[Qemu-devel] [PATCH 0/4] spice: improve vga mode performance
Hi, This patch series makes spice be more clever on screen updates when in vga mode (i.e. without qxl guest driver loaded or when using a non-qxl vga card). qemu keeps a copy of the screen with the content forwarded to spice-server, then on updates it goes compare the guest screen with the copy to figure what has really changed. cheers, Gerd Gerd Hoffmann (4): spice: switch to queue for vga mode updates spice: split qemu_spice_create_update spice: add screen mirror spice: send updates only for changed screen content hw/qxl.c |6 +- ui/spice-display.c | 135 ui/spice-display.h |4 +- 3 files changed, 111 insertions(+), 34 deletions(-)
[Qemu-devel] [PATCH 1/4] spice: switch to queue for vga mode updates
---
hw/qxl.c |6 +++---
ui/spice-display.c | 25 ++---
ui/spice-display.h |3 ++-
3 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index b726c19..833cd77 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -597,9 +597,9 @@ static int interface_get_command(QXLInstance *sin, struct
QXLCommandExt *ext)
case QXL_MODE_VGA:
ret = false;
qemu_mutex_lock(qxl-ssd.lock);
-if (qxl-ssd.update != NULL) {
-update = qxl-ssd.update;
-qxl-ssd.update = NULL;
+update = QTAILQ_FIRST(qxl-ssd.updates);
+if (update != NULL) {
+QTAILQ_REMOVE(qxl-ssd.updates, update, next);
*ext = update-ext;
ret = true;
}
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 99bc665..59c5fd7 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -164,7 +164,7 @@ int qemu_spice_display_is_running(SimpleSpiceDisplay *ssd)
#endif
}
-static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
+static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
{
SimpleSpiceUpdate *update;
QXLDrawable *drawable;
@@ -175,7 +175,7 @@ static SimpleSpiceUpdate
*qemu_spice_create_update(SimpleSpiceDisplay *ssd)
struct timespec time_space;
if (qemu_spice_rect_is_empty(ssd-dirty)) {
-return NULL;
+return;
};
trace_qemu_spice_create_update(
@@ -239,7 +239,7 @@ static SimpleSpiceUpdate
*qemu_spice_create_update(SimpleSpiceDisplay *ssd)
cmd-data = (uintptr_t)drawable;
memset(ssd-dirty, 0, sizeof(ssd-dirty));
-return update;
+QTAILQ_INSERT_TAIL(ssd-updates, update, next);
}
/*
@@ -315,6 +315,7 @@ void qemu_spice_display_init_common(SimpleSpiceDisplay
*ssd, DisplayState *ds)
{
ssd-ds = ds;
qemu_mutex_init(ssd-lock);
+QTAILQ_INIT(ssd-updates);
ssd-mouse_x = -1;
ssd-mouse_y = -1;
if (ssd-num_surfaces == 0) {
@@ -345,6 +346,8 @@ void qemu_spice_display_update(SimpleSpiceDisplay *ssd,
void qemu_spice_display_resize(SimpleSpiceDisplay *ssd)
{
+SimpleSpiceUpdate *update;
+
dprint(1, %s:\n, __FUNCTION__);
memset(ssd-dirty, 0, sizeof(ssd-dirty));
@@ -352,9 +355,9 @@ void qemu_spice_display_resize(SimpleSpiceDisplay *ssd)
ssd-conv = NULL;
qemu_mutex_lock(ssd-lock);
-if (ssd-update != NULL) {
-qemu_spice_destroy_update(ssd, ssd-update);
-ssd-update = NULL;
+while ((update = QTAILQ_FIRST(ssd-updates)) != NULL) {
+QTAILQ_REMOVE(ssd-updates, update, next);
+qemu_spice_destroy_update(ssd, update);
}
qemu_mutex_unlock(ssd-lock);
qemu_spice_destroy_host_primary(ssd);
@@ -384,8 +387,8 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
vga_hw_update();
qemu_mutex_lock(ssd-lock);
-if (ssd-update == NULL) {
-ssd-update = qemu_spice_create_update(ssd);
+if (QTAILQ_EMPTY(ssd-updates)) {
+qemu_spice_create_update(ssd);
ssd-notify++;
}
qemu_spice_cursor_refresh_unlocked(ssd);
@@ -442,9 +445,9 @@ static int interface_get_command(QXLInstance *sin, struct
QXLCommandExt *ext)
dprint(3, %s:\n, __FUNCTION__);
qemu_mutex_lock(ssd-lock);
-if (ssd-update != NULL) {
-update = ssd-update;
-ssd-update = NULL;
+update = QTAILQ_FIRST(ssd-updates);
+if (update != NULL) {
+QTAILQ_REMOVE(ssd-updates, update, next);
*ext = update-ext;
ret = true;
}
diff --git a/ui/spice-display.h b/ui/spice-display.h
index 512ab78..3fcb6fe 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -92,7 +92,7 @@ struct SimpleSpiceDisplay {
* to them must be protected by the lock.
*/
QemuMutex lock;
-SimpleSpiceUpdate *update;
+QTAILQ_HEAD(, SimpleSpiceUpdate) updates;
QEMUCursor *cursor;
int mouse_x, mouse_y;
};
@@ -102,6 +102,7 @@ struct SimpleSpiceUpdate {
QXLImage image;
QXLCommandExt ext;
uint8_t *bitmap;
+QTAILQ_ENTRY(SimpleSpiceUpdate) next;
};
int qemu_spice_rect_is_empty(const QXLRect* r);
--
1.7.1
Re: [Qemu-devel] [Qemu-trivial] [PATCH] block: output more error messages if failed to create temporary snapshot
Am 05.09.2012 11:40, schrieb Stefan Hajnoczi:
On Wed, Sep 05, 2012 at 04:26:14PM +0800, riegama...@gmail.com wrote:
From: Dunrong Huang riegama...@gmail.com
If we failed to create temporary snapshot, the error message did not match
with the error, for example:
$ TMPDIR=/tmp/bad_path qemu-system-x86_64 -enable-kvm debian.qcow2 -snapshot
qemu-system-x86_64: -enable-kvm: could not open disk image
/home/mathslinux/Images/debian.qcow2: No such file or directory
Indeed, the file which cant be created is /tmp/bad_path/vl.xx, not
debian.qcow2. so the error message makes users feel confused.
Signed-off-by: Dunrong Huang riegama...@gmail.com
---
block.c | 2 ++
1 个文件被修改，插入 2 行(+)
diff --git a/block.c b/block.c
index 470bdcc..c9e5140 100644
--- a/block.c
+++ b/block.c
@@ -434,6 +434,8 @@ int get_tmp_filename(char *filename, int size)
}
fd = mkstemp(filename);
if (fd 0 || close(fd)) {
+fprintf(stderr, Could not create temporary snapshot in %s
directory:
+%s\n, tmpdir, strerror(errno));
The error message is fine for fd 0 but not for close(0) != 0. Also,
close(2) is allowed to change errno (even on success) so this is not
portable and could clobber the errno value.
I don't think this error message is fine in get_tmp_filename(). This
function happens to be used for temporary snapshots, but this is not
part of its interface. Today it's also used in vvfat and there the
message would only be confusing. Other use cases may be introduced in
the future.
If you want to introduce an error message, do it in the caller.
Kevin
Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem
On 2012-09-05 11:52, Avi Kivity wrote: On 09/05/2012 11:19 AM, liu ping fan wrote: [...] We could externalize the refcounting and push it into device code. This means: memory_region_init_io(s-mem, dev) ... object_ref(dev) memory_region_add_subregion(..., dev-mr) ... memory_region_del_subregion(..., dev-mr) // implied flush object_unref(dev) I think object_ref(dev) just another style to push MemoryRegionOps::object() to device level. And I think we can bypass it. The caller (unplug, pci-reconfig ) of memory_region_del_subregion() ensure the @dev is valid. If the implied flush is implemented in synchronize, _del_subregion() will guarantee no reader for @dev-mr and @dev exist any longer. The above code has a deadlock. memory_region_del_subregion() may be called under the device lock (since it may be the result of mmio to the device), and if the flush uses synchronized_rcu(), it will wait forever for the read-side critical section to complete. But if _del_subregion() just wait for mr-X quiescent period, while calling in mr-Y's read side critical section, then we can avoid deadlock. I saw in pci-mapping, we delete mr-X in mr-Y read side. So I think we can save both object_ref/unref(dev) for memory system. The only problem is that whether we can implement it as synchronous or not, is it possible that we can launch a _del_subregion(mr-X) in mr-X's dispatcher? Yes. Real cases exist. Oh, I find the sample code, then, the deadlock is unavoidable in this method. What alternatives remain? I think a way out may be async+refcnt If we consider the relationship of MemoryRegionImpl and device as the one between file and file-private_data in Linux. Then the creation of impl will object_ref(dev) and when impl-ref=0, it will object_unref(dev) But this is an async model, for those client which need to know the end of flush, we can adopt callback just like call_rcu(). During this thread, it seems that no matter we adopt refcnt on MemoryRegionImpl or not, protecting MemoryRegion::opaque during dispatch is still required. Right. So MemoryRegionImpl is pointless. My current preference is MemoryRegionOps::ref(MemoryRegion *mr) (and a corresponding unref), which has the following requirements: - if the refcount is nonzero, MemoryRegion::opaque is safe to use - if the refcount is nonzero, the MemoryRegion itself is stable. The second point means that the memory subsystem will cache the region state and apply changes only after leaving a handler that performed them? Later we can change other callbacks to also use mr instead of opaque. Usually the callback will be able to derive the device object from the region, only special cases will require memory_region_opaque(MemoryRegion *). We can even drop the opaque from memory_region_init_io() and replace it with memory_region_set_opaque(), to be used in those special cases. It is challenging to substitute memory_region_init_io() 's 3rd parameter from void* to Object*, owning to the conflict that life-cycle management need the host of opaque, while Object and mr need 1:1 map. So I think, I will move forward on the way based on MemoryRegionOps::object(). Right? As outlined above, I now prefer MemoryRegionOps::ref/unref. Advantages compared to MemoryRegionOps::object(): - decoupled from the QOM framework Disadvantages: - more boilerplate code in converted devices Since we are likely to convert few devices to lockless dispatch, I believe the tradeoff is justified. But let's hear Jan and the others. I still need to dig into related posts of the past days, lost track, but the above sounds much better. Besides the question of what to protect and how, one important thing IMHO is that we are able to switch locking behaviour on a per region basis. And that should also be feasible this way. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH v6 2/2] block: Support GlusterFS as a QEMU block backend
On Wed, Sep 05, 2012 at 12:01:58PM +0200, Kevin Wolf wrote:
Am 05.09.2012 09:41, schrieb Bharata B Rao:
On Thu, Aug 09, 2012 at 06:32:16PM +0530, Bharata B Rao wrote:
+static void qemu_gluster_complete_aio(GlusterAIOCB *acb)
+{
+int ret;
+
+if (acb-canceled) {
+qemu_aio_release(acb);
+return;
+}
+
+if (acb-ret == acb-size) {
+ret = 0; /* Success */
+} else if (acb-ret 0) {
+ret = acb-ret; /* Read/Write failed */
+} else {
+ret = -EIO; /* Partial read/write - fail it */
+}
+acb-common.cb(acb-common.opaque, ret);
The .cb() here is bdrv_co_io_em_complete(). It does qemu_coroutine_enter(),
handles the return value and comes back here.
Right.
.cb is set by qemu_gluster_aio_rw/flush(), and the only way these can be
called is through bdrv_co_io_em() and bdrv_co_flush(), which both set
bdrv_co_io_em_complete as the callback.
Right.
But if the bdrv_read or bdrv_write or bdrv_flush was called from a
coroutine context (as against they themselves creating a new coroutine),
the above .cb() call above doesn't return to this point.
Why?
Note that in this particular scenario (qemu-img create -f qcow2), bdrv_read
and bdrv_write are called from the coroutine thread that is running
qcow2_create(). So bdrv_read will find itself running in coroutine context
and hence will continue to use the same coroutine thread.
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
bdrv_rw_co_entry(rwco);
}
The path taken is.
bdrv_rw_co_entry - bdrv_co_do_readv - bdrv_co_readv_em - bdrv_co_io_em
- qemu_gluster_aio_readv
bdrv_co_io_em does qemu_coroutine_yield() next.
When the AIO is completed, qemu_gluster_complete_aio() is run as the read end
of the pipe becomes ready, so I assume it is in non-coroutine context to start
with. When it does acb-common.cb(), it enters the co-routine which was yielded
by bdrv_co_io_em.
Now the read call returns back and we ultimately end up in bdrv_rw_co_entry
which takes us back to bdrv_read and back to bdrv_pwrite where all this
originated (Note that qcow2_create2 called bdrv_pwrite in the first place).
So I never come back to the next statement in qemu_gluster_complete_aio()
after acb-common.cb(acb-common.opaque, ret). So the coroutine didn't
end and continued futher by issuing another bdrv_write call.
The effect of this is seen next when qcow2_create calls bdrv_close which does
bdrv_drain_all which calls qemu_aio_wait and I never come out of it.
In qemu_aio_wait, node-io_flush(node-opaque) returns a non-zero value
always, because node-io_flush which is qemu_gluster_aio_flush_cb() returns
non zero always. This is happening since I never got a chance to decrement
s-qemu_aio_count which was supposed to happen after qemu_gluster_complete_aio
came back from .cb() call.
So this is what I think is happening, hoping that I got it right.
Note that when I schedule a BH in qemu_gluster_complete_aio(), then
things work fine apparently because I am able to continue and decrement
s-qemu_aio_count.
Regards,
Bharata.
Re: [Qemu-devel] [PATCH 0/2] slirp fixes
On 2012-09-04 23:20, Stefan Weil wrote: These patches replace http://patchwork.ozlabs.org/patch/181411/. I modified the code as suggested by Jan Kiska and separated the original patch in two patches. [PATCH 1/2] slirp: Remove wrong type casts ins debug statements [PATCH 2/2] slirp: Fix error reported by static code analysis Thanks, applied to slirp queue. And I'll propose patch 2 for stable as well. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 2/2] slirp: Fix error reported by static code analysis
On 2012-09-04 23:20, Stefan Weil wrote:
Report from smatch:
slirp/tcp_subr.c:127 tcp_respond(17) error:
we previously assumed 'tp' could be null (see line 124)
Return if 'tp' is NULL.
Signed-off-by: Stefan Weil s...@weilnetz.de
---
slirp/tcp_subr.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
index 5890d7a..1542e43 100644
--- a/slirp/tcp_subr.c
+++ b/slirp/tcp_subr.c
@@ -124,7 +124,7 @@ tcp_respond(struct tcpcb *tp, struct tcpiphdr *ti, struct
mbuf *m,
if (tp)
win = sbspace(tp-t_socket-so_rcv);
if (m == NULL) {
- if ((m = m_get(tp-t_socket-slirp)) == NULL)
+ if (!tp || (m = m_get(tp-t_socket-slirp)) == NULL)
return;
tlen = 0;
m-m_data += IF_MAXLINKHDR;
I suppose this is also stable material, therefore extending CC.
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] fix entry pointer for ELF kernels loaded with -kernel option
Am 05.09.2012 11:05, schrieb Henning Schild: This patch fixes a bug in qemu which prevents Multiboot ELF kernels from being loaded with the -kernel option. Find a full description of the problem here https://bugs.launchpad.net/qemu/+bug/1044727 . The logic looks good to me, but there are a few points about the patch itself (see http://wiki.qemu.org/Contribute/SubmitAPatch). First thing is that the patch needs a proper Signed-off-by line. This is absolutely crucial. The other points could be fixed manually by a potential patient enough maintainer, but you are the only one who can provide the Signed-off-by. Without it, the patch won't be applied. --- hw/elf_ops.h | 10 ++ 1 files changed, 10 insertions(+), 0 deletions(-) diff --git a/hw/elf_ops.h b/hw/elf_ops.h index fa65ce2..aeddd11 100644 --- a/hw/elf_ops.h +++ b/hw/elf_ops.h @@ -269,6 +269,16 @@ static int glue(load_elf, SZ)(const char *name, int fd, The patch is corrupted by line wraps. Using git format-patch/send-email avoids this kind of problems. Alternatively, attach the patch in addition so that an uncorrupted version can be used for applying it. addr = ph-p_paddr; } +/* the entry pointer in the ELF header is a virtual + * address, if the text segments paddr and vaddr differ + * we need to adjust the entry */ +if (pentry !translate_fn +ph-p_vaddr != ph-p_paddr +ehdr.e_entry = ph-p_vaddr +ehdr.e_entry ph-p_vaddr + ph-p_filesz +ph-p_flags PF_X) +*pentry = ehdr.e_entry - ph-p_vaddr + ph-p_paddr; The coding style problem that was already mentioned. qemu puts braces even for single statements. Kevin
Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem
On 09/05/2012 01:36 PM, Jan Kiszka wrote: My current preference is MemoryRegionOps::ref(MemoryRegion *mr) (and a corresponding unref), which has the following requirements: - if the refcount is nonzero, MemoryRegion::opaque is safe to use - if the refcount is nonzero, the MemoryRegion itself is stable. The second point means that the memory subsystem will cache the region state and apply changes only after leaving a handler that performed them? No. I/O callbacks may be called after a region has been disabled. I guess we can restrict this to converted regions, so we don't introduce regressions. As outlined above, I now prefer MemoryRegionOps::ref/unref. Advantages compared to MemoryRegionOps::object(): - decoupled from the QOM framework Disadvantages: - more boilerplate code in converted devices Since we are likely to convert few devices to lockless dispatch, I believe the tradeoff is justified. But let's hear Jan and the others. I still need to dig into related posts of the past days, lost track, but the above sounds much better. Besides the question of what to protect and how, one important thing IMHO is that we are able to switch locking behaviour on a per region basis. And that should also be feasible this way. It was also possible with MemoryRegionOps::object() - no one said all regions for a device have to refer to the same object (and there is a difference between locking and reference counting, each callback could take a different lock). But it is more natural with MemoryRegionOps::ref(). -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem
On 2012-09-05 12:53, Avi Kivity wrote:
On 09/05/2012 01:36 PM, Jan Kiszka wrote:
My current preference is MemoryRegionOps::ref(MemoryRegion *mr) (and a
corresponding unref), which has the following requirements:
- if the refcount is nonzero, MemoryRegion::opaque is safe to use
- if the refcount is nonzero, the MemoryRegion itself is stable.
The second point means that the memory subsystem will cache the region
state and apply changes only after leaving a handler that performed them?
No. I/O callbacks may be called after a region has been disabled.
I guess we can restrict this to converted regions, so we don't introduce
regressions.
s/can/have to/. This property change will require some special care,
hopefully mostly at the memory layer. A simple scenario from recent patches:
if (enable) {
memory_region_set_address(s-pm_io, pm_io_base);
memory_region_set_enabled(s-pm_io, true);
} else {
memory_region_set_enabled(s-pm_io, false);
}
We will have to ensure that set_enabled(..., true) will never cause a
dispatch using an outdated base address.
I think discussing semantics and usage patterns of the new memory API -
outside of the BQL - will be the next big topic. ;)
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem
On 09/05/2012 02:11 PM, Jan Kiszka wrote:
On 2012-09-05 12:53, Avi Kivity wrote:
On 09/05/2012 01:36 PM, Jan Kiszka wrote:
My current preference is MemoryRegionOps::ref(MemoryRegion *mr) (and a
corresponding unref), which has the following requirements:
- if the refcount is nonzero, MemoryRegion::opaque is safe to use
- if the refcount is nonzero, the MemoryRegion itself is stable.
The second point means that the memory subsystem will cache the region
state and apply changes only after leaving a handler that performed them?
No. I/O callbacks may be called after a region has been disabled.
I guess we can restrict this to converted regions, so we don't introduce
regressions.
s/can/have to/. This property change will require some special care,
hopefully mostly at the memory layer. A simple scenario from recent patches:
if (enable) {
memory_region_set_address(s-pm_io, pm_io_base);
memory_region_set_enabled(s-pm_io, true);
} else {
memory_region_set_enabled(s-pm_io, false);
}
I am unable to avoid pointing out that this can be collapsed to
memory_region_set_address(s-pm_io, pm_io_base);
memory_region_set_enabled(s-pm_io, enable);
as the address is meaningless when disabled. Sorry.
We will have to ensure that set_enabled(..., true) will never cause a
dispatch using an outdated base address.
No, this is entirely safe. If the guest changes a region offset
concurrently with issuing mmio on it, then it must expect either the old
or new offset to be used during dispatch. In either case, the correct
intra-region offset will be provided to the I/O callback (no volatile
MemoryRegion fields except -readable (IIRC) are used during dispatch -
the rest are all copied into data structures used during dispatch (this
is part of what makes the whole thing so rcu friendly).
I think discussing semantics and usage patterns of the new memory API -
outside of the BQL - will be the next big topic. ;)
I hope it won't prove to be that complicated.
--
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] boot device order has no effect for virtio-scsi devices
On 09/05/2012 02:55 PM, Paolo Bonzini wrote: Il 05/09/2012 00:13, ching ha scritto: Ah, ok. libvirt for now supports only booting from LUN 0. You can use ^^^ Oops, should have been SeaBIOS. multiple targets instead of multiple units. Paolo it works. thanks a lot. hope that libvirt will document it somewhere Please raise this issue on the libvirt lists, then. actually, why is the problem related to libvirt, not qemu, nor seabios? libvirt has generated the qemu command line with bootindex already. Yes, it's SeaBIOS. It is documented in the code. Paolo thank you very much for your kindly help. ching
Re: [Qemu-devel] [RFC 6/6] i386: -cpu help: remove reference to specific CPUID leaves/registers
On Fri, 17 Aug 2012 14:53:42 -0300
Eduardo Habkost ehabk...@redhat.com wrote:
The -cpu configuration interface is based on a list of feature names or
properties, on a single namespace, so there's no need to mention on
which CPUID leaf/register each flag is located.
Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
target-i386/cpu.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index a71c2fc..0a03c80 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1297,13 +1297,13 @@ void x86_cpu_list(FILE *f, fprintf_function
cpu_fprintf) }
(*cpu_fprintf)(f, \nRecognized CPUID flags:\n);
listflags(buf, sizeof(buf), (uint32_t)~0, feature_name, 1);
-(*cpu_fprintf)(f, f_edx: %s\n, buf);
+(*cpu_fprintf)(f, %s\n, buf);
listflags(buf, sizeof(buf), (uint32_t)~0, ext_feature_name, 1);
-(*cpu_fprintf)(f, f_ecx: %s\n, buf);
+(*cpu_fprintf)(f, %s\n, buf);
listflags(buf, sizeof(buf), (uint32_t)~0, ext2_feature_name, 1);
-(*cpu_fprintf)(f, extf_edx: %s\n, buf);
+(*cpu_fprintf)(f, %s\n, buf);
listflags(buf, sizeof(buf), (uint32_t)~0, ext3_feature_name, 1);
-(*cpu_fprintf)(f, extf_ecx: %s\n, buf);
+(*cpu_fprintf)(f, %s\n, buf);
}
CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
If you are removing this ones, why do you leave:
if (dump) {
...
(*cpu_fprintf)(f, feature_edx %08x (%s)\n, def-features,
Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem
On 2012-09-05 13:25, Avi Kivity wrote:
On 09/05/2012 02:11 PM, Jan Kiszka wrote:
On 2012-09-05 12:53, Avi Kivity wrote:
On 09/05/2012 01:36 PM, Jan Kiszka wrote:
My current preference is MemoryRegionOps::ref(MemoryRegion *mr) (and a
corresponding unref), which has the following requirements:
- if the refcount is nonzero, MemoryRegion::opaque is safe to use
- if the refcount is nonzero, the MemoryRegion itself is stable.
The second point means that the memory subsystem will cache the region
state and apply changes only after leaving a handler that performed them?
No. I/O callbacks may be called after a region has been disabled.
I guess we can restrict this to converted regions, so we don't introduce
regressions.
s/can/have to/. This property change will require some special care,
hopefully mostly at the memory layer. A simple scenario from recent patches:
if (enable) {
memory_region_set_address(s-pm_io, pm_io_base);
memory_region_set_enabled(s-pm_io, true);
} else {
memory_region_set_enabled(s-pm_io, false);
}
I am unable to avoid pointing out that this can be collapsed to
memory_region_set_address(s-pm_io, pm_io_base);
memory_region_set_enabled(s-pm_io, enable);
as the address is meaningless when disabled. Sorry.
We will have to ensure that set_enabled(..., true) will never cause a
dispatch using an outdated base address.
No, this is entirely safe. If the guest changes a region offset
concurrently with issuing mmio on it, then it must expect either the old
or new offset to be used during dispatch.
You assume disable, reprogram, enable, ignoring that there could be
multiple, invalid states between disable and enable. Real hardware takes
care of the ordering.
In either case, the correct
intra-region offset will be provided to the I/O callback (no volatile
MemoryRegion fields except -readable (IIRC) are used during dispatch -
the rest are all copied into data structures used during dispatch (this
is part of what makes the whole thing so rcu friendly).
I think discussing semantics and usage patterns of the new memory API -
outside of the BQL - will be the next big topic. ;)
I hope it won't prove to be that complicated.
Yeah, let's hope this...
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem
On 09/05/2012 03:02 PM, Jan Kiszka wrote:
On 2012-09-05 13:25, Avi Kivity wrote:
On 09/05/2012 02:11 PM, Jan Kiszka wrote:
On 2012-09-05 12:53, Avi Kivity wrote:
On 09/05/2012 01:36 PM, Jan Kiszka wrote:
My current preference is MemoryRegionOps::ref(MemoryRegion *mr) (and a
corresponding unref), which has the following requirements:
- if the refcount is nonzero, MemoryRegion::opaque is safe to use
- if the refcount is nonzero, the MemoryRegion itself is stable.
The second point means that the memory subsystem will cache the region
state and apply changes only after leaving a handler that performed them?
No. I/O callbacks may be called after a region has been disabled.
I guess we can restrict this to converted regions, so we don't introduce
regressions.
s/can/have to/. This property change will require some special care,
hopefully mostly at the memory layer. A simple scenario from recent patches:
if (enable) {
memory_region_set_address(s-pm_io, pm_io_base);
memory_region_set_enabled(s-pm_io, true);
} else {
memory_region_set_enabled(s-pm_io, false);
}
I am unable to avoid pointing out that this can be collapsed to
memory_region_set_address(s-pm_io, pm_io_base);
memory_region_set_enabled(s-pm_io, enable);
as the address is meaningless when disabled. Sorry.
We will have to ensure that set_enabled(..., true) will never cause a
dispatch using an outdated base address.
No, this is entirely safe. If the guest changes a region offset
concurrently with issuing mmio on it, then it must expect either the old
or new offset to be used during dispatch.
I forgot to mention that my clever rewrite above actually breaks this -
it needs to be wrapped in a transaction, otherwise we have a
move-then-disable pattern.
You assume disable, reprogram, enable, ignoring that there could be
multiple, invalid states between disable and enable. Real hardware takes
care of the ordering.
It's possible of course, but the snippet above isn't susceptible on its own.
I don't think it's solvable. To serialize, you must hold the device
lock, but we don't want to take the device lock during dispatch.
Users can protect against them by checking for -enabled:
void foo_io_read(...)
{
FooState *s = container_of(mr, ...);
qemu_mutex_lock(s-lock);
if (!memory_region_enabled(mr)) {
*data = ~(uint64_t)0;
goto out;
}
...
out:
qemu_mutex_unlock(s-lock);
}
Non-converted users will be naturally protected since we will take the
bql on their behalf.
--
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 1/7] block: correctly set the keep_read_only flag
Am 30.08.2012 20:47, schrieb Jeff Cody:
I believe the bs-keep_read_only flag is supposed to reflect
the initial open state of the device. If the device is initially
opened R/O, then commit operations, or reopen operations changing
to R/W, are prohibited.
Currently, the keep_read_only flag is only accurate for the active
layer, and its backing file. Subsequent images end up always having
the keep_read_only flag set.
For instance, what happens now:
[ base ] kro = 1, ro = 1
|
v
[ snap-1 ] kro = 1, ro = 1
|
v
[ snap-2 ] kro = 0, ro = 1
|
v
[ active ] kro = 0, ro = 0
What we want:
[ base ] kro = 0, ro = 1
|
v
[ snap-1 ] kro = 0, ro = 1
|
v
[ snap-2 ] kro = 0, ro = 1
|
v
[ active ] kro = 0, ro = 0
Signed-off-by: Jeff Cody jc...@redhat.com
---
block.c | 16 +++-
block.h | 1 +
2 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/block.c b/block.c
index 470bdcc..e31b76f 100644
--- a/block.c
+++ b/block.c
@@ -655,7 +655,7 @@ static int bdrv_open_common(BlockDriverState *bs, const
char *filename,
* Clear flags that are internal to the block layer before opening the
* image.
*/
-open_flags = ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
+open_flags = ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING | BDRV_O_ALLOW_RDWR);
/*
* Snapshots should be writable.
@@ -664,8 +664,6 @@ static int bdrv_open_common(BlockDriverState *bs, const
char *filename,
open_flags |= BDRV_O_RDWR;
}
-bs-keep_read_only = bs-read_only = !(open_flags BDRV_O_RDWR);
-
/* Open the image, either directly or using a protocol */
if (drv-bdrv_file_open) {
ret = drv-bdrv_file_open(bs, filename, open_flags);
@@ -804,6 +802,12 @@ int bdrv_open(BlockDriverState *bs, const char
*filename, int flags,
goto unlink_and_fail;
}
+if (flags BDRV_O_RDWR) {
+flags |= BDRV_O_ALLOW_RDWR;
+}
+
+bs-keep_read_only = !(flags BDRV_O_ALLOW_RDWR);
Do we still need bs-keep_read_only or is it duplicated in
bs-open_flags now? We can convert all users in a follow-up patch.
Kevin
Re: [Qemu-devel] [Qemu-trivial] [PATCH] block: output more error messages if failed to create temporary snapshot
2012/9/5 Kevin Wolf kw...@redhat.com:
Am 05.09.2012 11:40, schrieb Stefan Hajnoczi:
On Wed, Sep 05, 2012 at 04:26:14PM +0800, riegama...@gmail.com wrote:
From: Dunrong Huang riegama...@gmail.com
If we failed to create temporary snapshot, the error message did not match
with the error, for example:
$ TMPDIR=/tmp/bad_path qemu-system-x86_64 -enable-kvm debian.qcow2 -snapshot
qemu-system-x86_64: -enable-kvm: could not open disk image
/home/mathslinux/Images/debian.qcow2: No such file or directory
Indeed, the file which cant be created is /tmp/bad_path/vl.xx, not
debian.qcow2. so the error message makes users feel confused.
Signed-off-by: Dunrong Huang riegama...@gmail.com
---
block.c | 2 ++
1 个文件被修改，插入 2 行(+)
diff --git a/block.c b/block.c
index 470bdcc..c9e5140 100644
--- a/block.c
+++ b/block.c
@@ -434,6 +434,8 @@ int get_tmp_filename(char *filename, int size)
}
fd = mkstemp(filename);
if (fd 0 || close(fd)) {
+fprintf(stderr, Could not create temporary snapshot in %s
directory:
+%s\n, tmpdir, strerror(errno));
The error message is fine for fd 0 but not for close(0) != 0. Also,
close(2) is allowed to change errno (even on success) so this is not
portable and could clobber the errno value.
I don't think this error message is fine in get_tmp_filename(). This
function happens to be used for temporary snapshots, but this is not
part of its interface. Today it's also used in vvfat and there the
message would only be confusing. Other use cases may be introduced in
the future.
Sorry, I forgot that get_tmp_filename() is a public interface.
If you want to introduce an error message, do it in the caller.
Kevin
--
Best Regards,
Dunrong Huang
Re: [Qemu-devel] [PATCH] fix entry pointer for ELF kernels loaded with -kernel option
Find a hopefully proper patch attached. Take it or leave it.
Signed-off-by: Henning Schild henn...@hennsch.de
---
hw/elf_ops.h | 11 +++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/hw/elf_ops.h b/hw/elf_ops.h
index fa65ce2..731a983 100644
--- a/hw/elf_ops.h
+++ b/hw/elf_ops.h
@@ -269,6 +269,17 @@ static int glue(load_elf, SZ)(const char *name,
int fd,
addr = ph-p_paddr;
}
+/* the entry pointer in the ELF header is a virtual
+ * address, if the text segments paddr and vaddr differ
+ * we need to adjust the entry */
+if (pentry !translate_fn
+ph-p_vaddr != ph-p_paddr
+ehdr.e_entry = ph-p_vaddr
+ehdr.e_entry ph-p_vaddr + ph-p_filesz
+ph-p_flags PF_X) {
+*pentry = ehdr.e_entry - ph-p_vaddr + ph-p_paddr;
+}
+
snprintf(label, sizeof(label), phdr #%d: %s, i, name);
rom_add_blob_fixed(label, data, mem_size, addr);
--
1.7.8.6
Am Mittwoch, den 05.09.2012, 12:53 +0200 schrieb Kevin Wolf
kw...@redhat.com:
Am 05.09.2012 11:05, schrieb Henning Schild:
This patch fixes a bug in qemu which prevents Multiboot ELF kernels
from being loaded with the -kernel option. Find a full description
of
the problem here https://bugs.launchpad.net/qemu/+bug/1044727 .
The logic looks good to me, but there are a few points about the
patch
itself (see http://wiki.qemu.org/Contribute/SubmitAPatch).
First thing is that the patch needs a proper Signed-off-by line. This
is
absolutely crucial. The other points could be fixed manually by a
potential patient enough maintainer, but you are the only one who can
provide the Signed-off-by. Without it, the patch won't be applied.
---
hw/elf_ops.h | 10 ++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/hw/elf_ops.h b/hw/elf_ops.h
index fa65ce2..aeddd11 100644
--- a/hw/elf_ops.h
+++ b/hw/elf_ops.h
@@ -269,6 +269,16 @@ static int glue(load_elf, SZ)(const char
*name,
int fd,
The patch is corrupted by line wraps. Using git
format-patch/send-email
avoids this kind of problems. Alternatively, attach the patch in
addition so that an uncorrupted version can be used for applying it.
addr = ph-p_paddr;
}
+/* the entry pointer in the ELF header is a virtual
+ * address, if the text segments paddr and vaddr
differ
+ * we need to adjust the entry */
+if (pentry !translate_fn
+ph-p_vaddr != ph-p_paddr
+ehdr.e_entry = ph-p_vaddr
+ehdr.e_entry ph-p_vaddr + ph-p_filesz
+ph-p_flags PF_X)
+*pentry = ehdr.e_entry - ph-p_vaddr +
ph-p_paddr;
The coding style problem that was already mentioned. qemu puts braces
even for single statements.
Kevin
From 0d3c07a78afa9ff77ca8f4ef90bd76b9fd7d948f Mon Sep 17 00:00:00 2001
From: Henning Schild henn...@hennsch.de
Date: Wed, 5 Sep 2012 14:46:38 +0200
Subject: [PATCH] fix entry pointer for ELF kernels loaded with -kernel option
Signed-off-by: Henning Schild henn...@hennsch.de
---
hw/elf_ops.h | 11 +++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/hw/elf_ops.h b/hw/elf_ops.h
index fa65ce2..731a983 100644
--- a/hw/elf_ops.h
+++ b/hw/elf_ops.h
@@ -269,6 +269,17 @@ static int glue(load_elf, SZ)(const char *name, int fd,
addr = ph-p_paddr;
}
+/* the entry pointer in the ELF header is a virtual
+ * address, if the text segments paddr and vaddr differ
+ * we need to adjust the entry */
+if (pentry !translate_fn
+ph-p_vaddr != ph-p_paddr
+ehdr.e_entry = ph-p_vaddr
+ehdr.e_entry ph-p_vaddr + ph-p_filesz
+ph-p_flags PF_X) {
+*pentry = ehdr.e_entry - ph-p_vaddr + ph-p_paddr;
+}
+
snprintf(label, sizeof(label), phdr #%d: %s, i, name);
rom_add_blob_fixed(label, data, mem_size, addr);
--
1.7.8.6
Re: [Qemu-devel] [PATCH 1/7] block: correctly set the keep_read_only flag
On 09/05/2012 08:47 AM, Kevin Wolf wrote:
Am 30.08.2012 20:47, schrieb Jeff Cody:
I believe the bs-keep_read_only flag is supposed to reflect
the initial open state of the device. If the device is initially
opened R/O, then commit operations, or reopen operations changing
to R/W, are prohibited.
Currently, the keep_read_only flag is only accurate for the active
layer, and its backing file. Subsequent images end up always having
the keep_read_only flag set.
For instance, what happens now:
[ base ] kro = 1, ro = 1
|
v
[ snap-1 ] kro = 1, ro = 1
|
v
[ snap-2 ] kro = 0, ro = 1
|
v
[ active ] kro = 0, ro = 0
What we want:
[ base ] kro = 0, ro = 1
|
v
[ snap-1 ] kro = 0, ro = 1
|
v
[ snap-2 ] kro = 0, ro = 1
|
v
[ active ] kro = 0, ro = 0
Signed-off-by: Jeff Cody jc...@redhat.com
---
block.c | 16 +++-
block.h | 1 +
2 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/block.c b/block.c
index 470bdcc..e31b76f 100644
--- a/block.c
+++ b/block.c
@@ -655,7 +655,7 @@ static int bdrv_open_common(BlockDriverState *bs, const
char *filename,
* Clear flags that are internal to the block layer before opening the
* image.
*/
-open_flags = ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
+open_flags = ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING |
BDRV_O_ALLOW_RDWR);
/*
* Snapshots should be writable.
@@ -664,8 +664,6 @@ static int bdrv_open_common(BlockDriverState *bs, const
char *filename,
open_flags |= BDRV_O_RDWR;
}
-bs-keep_read_only = bs-read_only = !(open_flags BDRV_O_RDWR);
-
/* Open the image, either directly or using a protocol */
if (drv-bdrv_file_open) {
ret = drv-bdrv_file_open(bs, filename, open_flags);
@@ -804,6 +802,12 @@ int bdrv_open(BlockDriverState *bs, const char
*filename, int flags,
goto unlink_and_fail;
}
+if (flags BDRV_O_RDWR) {
+flags |= BDRV_O_ALLOW_RDWR;
+}
+
+bs-keep_read_only = !(flags BDRV_O_ALLOW_RDWR);
Do we still need bs-keep_read_only or is it duplicated in
bs-open_flags now? We can convert all users in a follow-up patch.
I think we can ditch keep_read_only, and just use BDRV_O_ALLOW_RDWR. The
only other user of keep_read_only is the original bdrv_commit(), in
block.c. And, the natural way to convert that function would be to have
it use bdrv_reopen(), to change from r/o-r/w.
Re: [Qemu-devel] [PATCH] fix entry pointer for ELF kernels loaded with -kernel option
Am 05.09.2012 14:56, schrieb Henning Schild:
Find a hopefully proper patch attached. Take it or leave it.
Signed-off-by: Henning Schild henn...@hennsch.de
Reviewed-by: Kevin Wolf kw...@redhat.com
Aurelien, I think in the past you committed some changes in this area.
Does this look good to you and can you get it committed?
Kevin
---
hw/elf_ops.h | 11 +++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/hw/elf_ops.h b/hw/elf_ops.h
index fa65ce2..731a983 100644
--- a/hw/elf_ops.h
+++ b/hw/elf_ops.h
@@ -269,6 +269,17 @@ static int glue(load_elf, SZ)(const char *name,
int fd,
addr = ph-p_paddr;
}
+/* the entry pointer in the ELF header is a virtual
+ * address, if the text segments paddr and vaddr differ
+ * we need to adjust the entry */
+if (pentry !translate_fn
+ph-p_vaddr != ph-p_paddr
+ehdr.e_entry = ph-p_vaddr
+ehdr.e_entry ph-p_vaddr + ph-p_filesz
+ph-p_flags PF_X) {
+*pentry = ehdr.e_entry - ph-p_vaddr + ph-p_paddr;
+}
+
snprintf(label, sizeof(label), phdr #%d: %s, i, name);
rom_add_blob_fixed(label, data, mem_size, addr);
Re: [Qemu-devel] [PATCH 1/7] block: correctly set the keep_read_only flag
Am 05.09.2012 15:08, schrieb Jeff Cody:
On 09/05/2012 08:47 AM, Kevin Wolf wrote:
Am 30.08.2012 20:47, schrieb Jeff Cody:
I believe the bs-keep_read_only flag is supposed to reflect
the initial open state of the device. If the device is initially
opened R/O, then commit operations, or reopen operations changing
to R/W, are prohibited.
Currently, the keep_read_only flag is only accurate for the active
layer, and its backing file. Subsequent images end up always having
the keep_read_only flag set.
For instance, what happens now:
[ base ] kro = 1, ro = 1
|
v
[ snap-1 ] kro = 1, ro = 1
|
v
[ snap-2 ] kro = 0, ro = 1
|
v
[ active ] kro = 0, ro = 0
What we want:
[ base ] kro = 0, ro = 1
|
v
[ snap-1 ] kro = 0, ro = 1
|
v
[ snap-2 ] kro = 0, ro = 1
|
v
[ active ] kro = 0, ro = 0
Signed-off-by: Jeff Cody jc...@redhat.com
---
block.c | 16 +++-
block.h | 1 +
2 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/block.c b/block.c
index 470bdcc..e31b76f 100644
--- a/block.c
+++ b/block.c
@@ -655,7 +655,7 @@ static int bdrv_open_common(BlockDriverState *bs, const
char *filename,
* Clear flags that are internal to the block layer before opening the
* image.
*/
-open_flags = ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
+open_flags = ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING |
BDRV_O_ALLOW_RDWR);
/*
* Snapshots should be writable.
@@ -664,8 +664,6 @@ static int bdrv_open_common(BlockDriverState *bs, const
char *filename,
open_flags |= BDRV_O_RDWR;
}
-bs-keep_read_only = bs-read_only = !(open_flags BDRV_O_RDWR);
-
/* Open the image, either directly or using a protocol */
if (drv-bdrv_file_open) {
ret = drv-bdrv_file_open(bs, filename, open_flags);
@@ -804,6 +802,12 @@ int bdrv_open(BlockDriverState *bs, const char
*filename, int flags,
goto unlink_and_fail;
}
+if (flags BDRV_O_RDWR) {
+flags |= BDRV_O_ALLOW_RDWR;
+}
+
+bs-keep_read_only = !(flags BDRV_O_ALLOW_RDWR);
Do we still need bs-keep_read_only or is it duplicated in
bs-open_flags now? We can convert all users in a follow-up patch.
I think we can ditch keep_read_only, and just use BDRV_O_ALLOW_RDWR. The
only other user of keep_read_only is the original bdrv_commit(), in
block.c. And, the natural way to convert that function would be to have
it use bdrv_reopen(), to change from r/o-r/w.
Ah, yes, makes sense. So just using it in patch 2 and then converting
bdrv_commit() should be enough.
Kevin
[Qemu-devel] [PATCH v3] block: output more error messages if failed to create temporary snapshot
From: Dunrong Huang riegama...@gmail.com
If we failed to create temporary snapshot, the error message did not match
with the error, for example:
$ TMPDIR=/tmp/bad_path qemu-system-x86_64 -enable-kvm debian.qcow2 -snapshot
qemu-system-x86_64: -enable-kvm: could not open disk image
/home/mathslinux/Images/debian.qcow2: No such file or directory
Indeed, the file which cant be created is /tmp/bad_path/vl.xx, not
debian.qcow2. so the error message makes users feel confused.
Signed-off-by: Dunrong Huang riegama...@gmail.com
---
v1 - v2:
Output error message only if fd 0
v2 - v3:
Output error message in the caller of get_tmp_filename()
block.c | 2 ++
1 个文件被修改，插入 2 行(+)
diff --git a/block.c b/block.c
index 470bdcc..074987e 100644
--- a/block.c
+++ b/block.c
@@ -764,6 +764,8 @@ int bdrv_open(BlockDriverState *bs, const char *filename,
int flags,
ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename));
if (ret 0) {
+fprintf(stderr, Could not create temporary snapshot %s: %s\n,
+tmp_filename, strerror(errno));
return ret;
}
--
1.7.12
[Qemu-devel] [PATCH] block: Don't forget to delete temporary file
From: Dunrong Huang riegama...@gmail.com
The caller would not delete temporary file after failed get_tmp_filename().
Signed-off-by: Dunrong Huang riegama...@gmail.com
---
block.c | 6 +-
1 个文件被修改，插入 5 行(+)，删除 1 行(-)
diff --git a/block.c b/block.c
index 074987e..2bc9f75 100644
--- a/block.c
+++ b/block.c
@@ -433,7 +433,11 @@ int get_tmp_filename(char *filename, int size)
return -EOVERFLOW;
}
fd = mkstemp(filename);
-if (fd 0 || close(fd)) {
+if (fd 0) {
+return -errno;
+}
+if (close(fd) != 0) {
+unlink(filename);
return -errno;
}
return 0;
--
1.7.12
[Qemu-devel] [PATCH V7 0/2] Add JSON output to qemu-img info
This patchset add a JSON output mode to the qemu-img info command. It's a rewrite from scratch of the original patchset by Wenchao Xia following Anthony Liguori advices on JSON formating. the --output=(json|human) option is now mandatory on the command line. Benoît Canet (3): qapi: Add SnapshotInfo. qapi: Add ImageInfo. qemu-img: Add json output option to the info command. in v2: eblake: make some field optionals squash the two qapi patchs together fix a typo on vm_clock_nsec bcanet: fix a potential memory leak in v3: lcapitulino: remove unneeded test put '\n' at the end of json in printf statement drop the uneeded head pointer in collect_snapshots in v4: Wenchao Xia Kevin Wolf: -Refactor to separate rate ImageInfo collection from human printing. Kevin Wolf: -Use --output=(json|human). -make the two choice exclusive and print a message if none is specified. -cosmetic '=' alignement in collect snapshots. Benoît Canet: -add full-backing-filename to the ImageInfo structure (needed for human printing) -make ImageInfo-actual_size optional depending on the context. in v5: Eric Blake: -use a constant for getopt parsing to avoid future short options collision. -make the command default to --output=human. -fix spurious whitespace change. -split vm-clock-nsec in two fields vm-clock-sec and vm-clock-nsec. -declare JSON structure as Since 1.3 in v6: Blue Swirl: -Add missing const in getopt structure declaration. Eric Blake: -Remove spurious undef. -Use an enum instead of two boolean. in v7: Kevin Wolf: -Add missing documentation. -Change bogus comment about chained lists support in qapi. -Remove collect_backing_file_format and use bs-backing_format instead. -Remove uneeded if. -Change bogus printing logic for backing file format. -Rename Format enum to OutputFormat. -Use OFORMAT_HUMAN by default and get rid of uneeded else branch -Use error_report insted of fprintf. -Use a switch instead of ifs. Benoît Canet: Rename FORMAT_HUMAN and FORMAT_JSON to OFORMAT_JSON and OFORMAT_HUMAN Benoît Canet (2): qapi: Add SnapshotInfo and ImageInfo. qemu-img: Add json output option to the info command. Makefile |3 +- qapi-schema.json | 64 +++ qemu-img-cmds.hx |4 +- qemu-img.c | 232 +++--- qemu-img.texi|5 +- 5 files changed, 259 insertions(+), 49 deletions(-) -- 1.7.9.5
[Qemu-devel] [PATCH V7 2/2] qemu-img: Add json output option to the info command.
This option --output=[human|json] make qemu-img info output on
human or JSON representation at the choice of the user.
example:
{
snapshots: [
{
vm-clock-nsec: 637102488,
name: vm-20120821145509,
date-sec: 1345553709,
date-nsec: 220289000,
vm-clock-sec: 20,
id: 1,
vm-state-size: 96522745
},
{
vm-clock-nsec: 28210866,
name: vm-20120821154059,
date-sec: 1345556459,
date-nsec: 171392000,
vm-clock-sec: 46,
id: 2,
vm-state-size: 101208714
}
],
virtual-size: 1073741824,
filename: snap.qcow2,
cluster-size: 65536,
format: qcow2,
actual-size: 985587712,
dirty-flag: false
}
Signed-off-by: Benoit Canet ben...@irqsave.net
---
Makefile |3 +-
qemu-img-cmds.hx |4 +-
qemu-img.c | 232 +++---
qemu-img.texi|5 +-
4 files changed, 195 insertions(+), 49 deletions(-)
diff --git a/Makefile b/Makefile
index 1cd5bc8..971e92f 100644
--- a/Makefile
+++ b/Makefile
@@ -157,7 +157,8 @@ tools-obj-y = $(oslib-obj-y) $(trace-obj-y) qemu-tool.o
qemu-timer.o \
iohandler.o cutils.o iov.o async.o
tools-obj-$(CONFIG_POSIX) += compatfd.o
-qemu-img$(EXESUF): qemu-img.o $(tools-obj-y) $(block-obj-y)
+qemu-img$(EXESUF): qemu-img.o $(tools-obj-y) $(block-obj-y) $(qapi-obj-y) \
+ qapi-visit.o qapi-types.o
qemu-nbd$(EXESUF): qemu-nbd.o $(tools-obj-y) $(block-obj-y)
qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y) $(block-obj-y)
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 39419a0..0ef82e9 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -34,9 +34,9 @@ STEXI
ETEXI
DEF(info, img_info,
-info [-f fmt] filename)
+info [-f fmt] [--output=ofmt] filename)
STEXI
-@item info [-f @var{fmt}] @var{filename}
+@item info [-f @var{fmt}] [--output=@var{ofmt}] @var{filename}
ETEXI
DEF(snapshot, img_snapshot,
diff --git a/qemu-img.c b/qemu-img.c
index b41e670..399f384 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -21,12 +21,16 @@
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
+#include qapi-visit.h
+#include qapi/qmp-output-visitor.h
+#include qjson.h
#include qemu-common.h
#include qemu-option.h
#include qemu-error.h
#include osdep.h
#include sysemu.h
#include block_int.h
+#include getopt.h
#include stdio.h
#ifdef _WIN32
@@ -84,6 +88,7 @@ static void help(void)
'-p' show progress of command (only certain commands)\n
'-S' indicates the consecutive number of bytes that must contain
only zeros\n
for qemu-img to create a sparse image during conversion\n
+ '--output' takes the format in which the output must be done
(human or json)\n
\n
Parameters to check subcommand:\n
'-r' tries to repair any inconsistencies that are found during
the check.\n
@@ -1102,21 +1107,169 @@ static void dump_snapshots(BlockDriverState *bs)
g_free(sn_tab);
}
-static int img_info(int argc, char **argv)
+static void collect_snapshots(BlockDriverState *bs , ImageInfo *info)
+{
+int i, sn_count;
+QEMUSnapshotInfo *sn_tab = NULL;
+SnapshotInfoList *info_list, *cur_item = NULL;
+sn_count = bdrv_snapshot_list(bs, sn_tab);
+
+for (i = 0; i sn_count; i++) {
+info-has_snapshots = true;
+info_list = g_new0(SnapshotInfoList, 1);
+
+info_list-value= g_new0(SnapshotInfo, 1);
+info_list-value-id= g_strdup(sn_tab[i].id_str);
+info_list-value-name = g_strdup(sn_tab[i].name);
+info_list-value-vm_state_size = sn_tab[i].vm_state_size;
+info_list-value-date_sec = sn_tab[i].date_sec;
+info_list-value-date_nsec = sn_tab[i].date_nsec;
+info_list-value-vm_clock_sec = sn_tab[i].vm_clock_nsec / 10;
+info_list-value-vm_clock_nsec = sn_tab[i].vm_clock_nsec % 10;
+
+/* XXX: waiting for the qapi to support qemu-queue.h types */
+if (!cur_item) {
+info-snapshots = cur_item = info_list;
+} else {
+cur_item-next = info_list;
+cur_item = info_list;
+}
+
+}
+
+g_free(sn_tab);
+}
+
+static void dump_json_image_info(ImageInfo *info)
+{
+Error *errp = NULL;
+QString *str;
+QmpOutputVisitor *ov = qmp_output_visitor_new();
+QObject *obj;
+visit_type_ImageInfo(qmp_output_get_visitor(ov),
+ info, NULL, errp);
+obj = qmp_output_get_qobject(ov);
+str = qobject_to_json_pretty(obj);
+assert(str != NULL);
+printf(%s\n, qstring_get_str(str));
+qobject_decref(obj);
+qmp_output_visitor_cleanup(ov);
+QDECREF(str);
+}
+
+static void collect_image_info(BlockDriverState *bs,
+
[Qemu-devel] [Bug 1028908] Re: qemu-img fails to convert VMDK image
This should be fixed as of commit 65bd155c (included in 1.2.0-rc1). -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1028908 Title: qemu-img fails to convert VMDK image Status in QEMU: New Bug description: Download raw learn_puppet_centos_pe2.5.1_ovf.2012.04.18-disk1.vmdk [1] then try to convert it to a more suitable format for qemu-kvm, e.g. raw: $ qemu-img convert -O raw learn_puppet_centos_pe2.5.1_ovf.2012.04.18-disk1.vmdk learn_puppet_centos_pe2.5.1_ovf.2012.04.18-disk1.raw qemu-img: error while reading sector 131072: Invalid argument The error happens with both qemu-img-1.0-17.fc17.x86_64 and the latest git master version (c3cdc1b0ff84d1cfed0c8052d2c83f8ecbf24166; thanks go to kwolf for testing it). Additional info: $ qemu-img info learn_puppet_centos_pe2.5.1_ovf.2012.04.18-disk1.vmdk image: learn_puppet_centos_pe2.5.1_ovf.2012.04.18-disk1.vmdk file format: vmdk virtual size: 4.0G (4294967296 bytes) disk size: 537M [1] http://downloads.puppetlabs.com/learning/learn_puppet_centos_pe2.5.1_ovf.2012.04.18.zip (516 MB) To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1028908/+subscriptions
[Qemu-devel] [PATCH V8 1/2] qapi: Add SnapshotInfo and ImageInfo.
Signed-off-by: Benoit Canet ben...@irqsave.net
---
qapi-schema.json | 64 ++
1 file changed, 64 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json
index bd8ad74..8723795 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -156,6 +156,70 @@
'running', 'save-vm', 'shutdown', 'suspended', 'watchdog' ] }
##
+# @SnapshotInfo
+#
+# @id: unique snapshot id
+#
+# @name: user chosen name
+#
+# @vm-state-size: size of the VM state
+#
+# @date-sec: UTC date of the snapshot in seconds
+#
+# @date-nsec: fractional part in nano seconds to be used with date-sec
+#
+# @vm-clock-sec: VM clock relative to boot in seconds
+#
+# @vm-clock-nsec: fractional part in nano seconds to be used with vm-clock-sec
+#
+# Since: 1.3
+#
+##
+
+{ 'type': 'SnapshotInfo',
+ 'data': { 'id': 'str', 'name': 'str', 'vm-state-size': 'int',
+'date-sec': 'int', 'date-nsec': 'int',
+'vm-clock-sec': 'int', 'vm-clock-nsec': 'int' } }
+
+##
+# @ImageInfo:
+#
+# Information about a QEMU image file
+#
+# @filename: name of the image file
+#
+# @format: format of the image file
+#
+# @virtual-size: maximum capacity in bytes of the image
+#
+# @actual-size: #optional actual size on disk in bytes of the image
+#
+# @dirty-flag: #optional true if image is not cleanly closed
+#
+# @cluster-size: #optional size of a cluster in bytes
+#
+# @encrypted: #optional true if the image is encrypted
+#
+# @backing-filename: #optional name of the backing file
+#
+# @full-backing-filename: #optional full path of the backing file
+#
+# @backing-filename-format: #optional the format of the backing file
+#
+# @snapshots: #optional list of VM snapshots
+#
+# Since: 1.3
+#
+##
+
+{ 'type': 'ImageInfo',
+ 'data': {'filename': 'str', 'format': 'str', '*dirty-flag': 'bool',
+ '*actual-size': 'int', 'virtual-size': 'int',
+ '*cluster-size': 'int', '*encrypted': 'bool',
+ '*backing-filename': 'str', '*full-backing-filename': 'str',
+ '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'] }
}
+
+##
# @StatusInfo:
#
# Information about VCPU run state
--
1.7.9.5
[Qemu-devel] [PATCH V8 2/2] qemu-img: Add json output option to the info command.
This option --output=[human|json] make qemu-img info output on
human or JSON representation at the choice of the user.
example:
{
snapshots: [
{
vm-clock-nsec: 637102488,
name: vm-20120821145509,
date-sec: 1345553709,
date-nsec: 220289000,
vm-clock-sec: 20,
id: 1,
vm-state-size: 96522745
},
{
vm-clock-nsec: 28210866,
name: vm-20120821154059,
date-sec: 1345556459,
date-nsec: 171392000,
vm-clock-sec: 46,
id: 2,
vm-state-size: 101208714
}
],
virtual-size: 1073741824,
filename: snap.qcow2,
cluster-size: 65536,
format: qcow2,
actual-size: 985587712,
dirty-flag: false
}
Signed-off-by: Benoit Canet ben...@irqsave.net
---
Makefile |3 +-
qemu-img-cmds.hx |4 +-
qemu-img.c | 236 --
qemu-img.texi|5 +-
4 files changed, 199 insertions(+), 49 deletions(-)
diff --git a/Makefile b/Makefile
index 1cd5bc8..971e92f 100644
--- a/Makefile
+++ b/Makefile
@@ -157,7 +157,8 @@ tools-obj-y = $(oslib-obj-y) $(trace-obj-y) qemu-tool.o
qemu-timer.o \
iohandler.o cutils.o iov.o async.o
tools-obj-$(CONFIG_POSIX) += compatfd.o
-qemu-img$(EXESUF): qemu-img.o $(tools-obj-y) $(block-obj-y)
+qemu-img$(EXESUF): qemu-img.o $(tools-obj-y) $(block-obj-y) $(qapi-obj-y) \
+ qapi-visit.o qapi-types.o
qemu-nbd$(EXESUF): qemu-nbd.o $(tools-obj-y) $(block-obj-y)
qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y) $(block-obj-y)
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 39419a0..0ef82e9 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -34,9 +34,9 @@ STEXI
ETEXI
DEF(info, img_info,
-info [-f fmt] filename)
+info [-f fmt] [--output=ofmt] filename)
STEXI
-@item info [-f @var{fmt}] @var{filename}
+@item info [-f @var{fmt}] [--output=@var{ofmt}] @var{filename}
ETEXI
DEF(snapshot, img_snapshot,
diff --git a/qemu-img.c b/qemu-img.c
index b41e670..30e33c7 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -21,12 +21,16 @@
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
+#include qapi-visit.h
+#include qapi/qmp-output-visitor.h
+#include qjson.h
#include qemu-common.h
#include qemu-option.h
#include qemu-error.h
#include osdep.h
#include sysemu.h
#include block_int.h
+#include getopt.h
#include stdio.h
#ifdef _WIN32
@@ -84,6 +88,7 @@ static void help(void)
'-p' show progress of command (only certain commands)\n
'-S' indicates the consecutive number of bytes that must contain
only zeros\n
for qemu-img to create a sparse image during conversion\n
+ '--output' takes the format in which the output must be done
(human or json)\n
\n
Parameters to check subcommand:\n
'-r' tries to repair any inconsistencies that are found during
the check.\n
@@ -1102,21 +1107,174 @@ static void dump_snapshots(BlockDriverState *bs)
g_free(sn_tab);
}
-static int img_info(int argc, char **argv)
+static void collect_snapshots(BlockDriverState *bs , ImageInfo *info)
+{
+int i, sn_count;
+QEMUSnapshotInfo *sn_tab = NULL;
+SnapshotInfoList *info_list, *cur_item = NULL;
+sn_count = bdrv_snapshot_list(bs, sn_tab);
+
+for (i = 0; i sn_count; i++) {
+info-has_snapshots = true;
+info_list = g_new0(SnapshotInfoList, 1);
+
+info_list-value= g_new0(SnapshotInfo, 1);
+info_list-value-id= g_strdup(sn_tab[i].id_str);
+info_list-value-name = g_strdup(sn_tab[i].name);
+info_list-value-vm_state_size = sn_tab[i].vm_state_size;
+info_list-value-date_sec = sn_tab[i].date_sec;
+info_list-value-date_nsec = sn_tab[i].date_nsec;
+info_list-value-vm_clock_sec = sn_tab[i].vm_clock_nsec / 10;
+info_list-value-vm_clock_nsec = sn_tab[i].vm_clock_nsec % 10;
+
+/* XXX: waiting for the qapi to support qemu-queue.h types */
+if (!cur_item) {
+info-snapshots = cur_item = info_list;
+} else {
+cur_item-next = info_list;
+cur_item = info_list;
+}
+
+}
+
+g_free(sn_tab);
+}
+
+static void dump_json_image_info(ImageInfo *info)
+{
+Error *errp = NULL;
+QString *str;
+QmpOutputVisitor *ov = qmp_output_visitor_new();
+QObject *obj;
+visit_type_ImageInfo(qmp_output_get_visitor(ov),
+ info, NULL, errp);
+obj = qmp_output_get_qobject(ov);
+str = qobject_to_json_pretty(obj);
+assert(str != NULL);
+printf(%s\n, qstring_get_str(str));
+qobject_decref(obj);
+qmp_output_visitor_cleanup(ov);
+QDECREF(str);
+}
+
+static void collect_image_info(BlockDriverState *bs,
+
[Qemu-devel] [Bug 1028908] Re: qemu-img fails to convert VMDK image
It works fine for me with git version de188751da8db3c77a681bf903035a0e5218c463 (it's post 1.2.0-rc1). ** Changed in: qemu Status: New = Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1028908 Title: qemu-img fails to convert VMDK image Status in QEMU: Fix Released Bug description: Download raw learn_puppet_centos_pe2.5.1_ovf.2012.04.18-disk1.vmdk [1] then try to convert it to a more suitable format for qemu-kvm, e.g. raw: $ qemu-img convert -O raw learn_puppet_centos_pe2.5.1_ovf.2012.04.18-disk1.vmdk learn_puppet_centos_pe2.5.1_ovf.2012.04.18-disk1.raw qemu-img: error while reading sector 131072: Invalid argument The error happens with both qemu-img-1.0-17.fc17.x86_64 and the latest git master version (c3cdc1b0ff84d1cfed0c8052d2c83f8ecbf24166; thanks go to kwolf for testing it). Additional info: $ qemu-img info learn_puppet_centos_pe2.5.1_ovf.2012.04.18-disk1.vmdk image: learn_puppet_centos_pe2.5.1_ovf.2012.04.18-disk1.vmdk file format: vmdk virtual size: 4.0G (4294967296 bytes) disk size: 537M [1] http://downloads.puppetlabs.com/learning/learn_puppet_centos_pe2.5.1_ovf.2012.04.18.zip (516 MB) To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1028908/+subscriptions
[Qemu-devel] [PATCH V7 1/2] qapi: Add SnapshotInfo and ImageInfo.
Signed-off-by: Benoit Canet ben...@irqsave.net
---
qapi-schema.json | 64 ++
1 file changed, 64 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json
index bd8ad74..8723795 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -156,6 +156,70 @@
'running', 'save-vm', 'shutdown', 'suspended', 'watchdog' ] }
##
+# @SnapshotInfo
+#
+# @id: unique snapshot id
+#
+# @name: user chosen name
+#
+# @vm-state-size: size of the VM state
+#
+# @date-sec: UTC date of the snapshot in seconds
+#
+# @date-nsec: fractional part in nano seconds to be used with date-sec
+#
+# @vm-clock-sec: VM clock relative to boot in seconds
+#
+# @vm-clock-nsec: fractional part in nano seconds to be used with vm-clock-sec
+#
+# Since: 1.3
+#
+##
+
+{ 'type': 'SnapshotInfo',
+ 'data': { 'id': 'str', 'name': 'str', 'vm-state-size': 'int',
+'date-sec': 'int', 'date-nsec': 'int',
+'vm-clock-sec': 'int', 'vm-clock-nsec': 'int' } }
+
+##
+# @ImageInfo:
+#
+# Information about a QEMU image file
+#
+# @filename: name of the image file
+#
+# @format: format of the image file
+#
+# @virtual-size: maximum capacity in bytes of the image
+#
+# @actual-size: #optional actual size on disk in bytes of the image
+#
+# @dirty-flag: #optional true if image is not cleanly closed
+#
+# @cluster-size: #optional size of a cluster in bytes
+#
+# @encrypted: #optional true if the image is encrypted
+#
+# @backing-filename: #optional name of the backing file
+#
+# @full-backing-filename: #optional full path of the backing file
+#
+# @backing-filename-format: #optional the format of the backing file
+#
+# @snapshots: #optional list of VM snapshots
+#
+# Since: 1.3
+#
+##
+
+{ 'type': 'ImageInfo',
+ 'data': {'filename': 'str', 'format': 'str', '*dirty-flag': 'bool',
+ '*actual-size': 'int', 'virtual-size': 'int',
+ '*cluster-size': 'int', '*encrypted': 'bool',
+ '*backing-filename': 'str', '*full-backing-filename': 'str',
+ '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'] }
}
+
+##
# @StatusInfo:
#
# Information about VCPU run state
--
1.7.9.5
[Qemu-devel] [PATCH V8 0/2] Add JSON output to qemu-img info
This patchset add a JSON output mode to the qemu-img info command. It's a rewrite from scratch of the original patchset by Wenchao Xia following Anthony Liguori advices on JSON formating. the --output=(json|human) option is now mandatory on the command line. Benoît Canet (3): qapi: Add SnapshotInfo. qapi: Add ImageInfo. qemu-img: Add json output option to the info command. in v2: eblake: make some field optionals squash the two qapi patchs together fix a typo on vm_clock_nsec bcanet: fix a potential memory leak in v3: lcapitulino: remove unneeded test put '\n' at the end of json in printf statement drop the uneeded head pointer in collect_snapshots in v4: Wenchao Xia Kevin Wolf: -Refactor to separate rate ImageInfo collection from human printing. Kevin Wolf: -Use --output=(json|human). -make the two choice exclusive and print a message if none is specified. -cosmetic '=' alignement in collect snapshots. Benoît Canet: -add full-backing-filename to the ImageInfo structure (needed for human printing) -make ImageInfo-actual_size optional depending on the context. in v5: Eric Blake: -use a constant for getopt parsing to avoid future short options collision. -make the command default to --output=human. -fix spurious whitespace change. -split vm-clock-nsec in two fields vm-clock-sec and vm-clock-nsec. -declare JSON structure as Since 1.3 in v6: Blue Swirl: -Add missing const in getopt structure declaration. Eric Blake: -Remove spurious undef. -Use an enum instead of two boolean. in v7: Kevin Wolf: -Add missing documentation. -Change bogus comment about chained lists support in qapi. -Remove collect_backing_file_format and use bs-backing_format instead. -Remove uneeded if. -Change bogus printing logic for backing file format. -Rename Format enum to OutputFormat. -Use OFORMAT_HUMAN by default and get rid of uneeded else branch -Use error_report insted of fprintf. -Use a switch instead of ifs. Benoît Canet: Rename FORMAT_HUMAN and FORMAT_JSON to OFORMAT_JSON and OFORMAT_HUMAN in v8: Kevin Wolf: -Check bs-backing_format[0] value to conditionaly include the format. -print the backing format if available in human output. -Move g_new0 down so we don't have the free it on error. -better documentation via @code usage. Benoît Canet (2): qapi: Add SnapshotInfo and ImageInfo. qemu-img: Add json output option to the info command. Makefile |3 +- qapi-schema.json | 64 +++ qemu-img-cmds.hx |4 +- qemu-img.c | 236 -- qemu-img.texi|5 +- 5 files changed, 263 insertions(+), 49 deletions(-) -- 1.7.9.5
Re: [Qemu-devel] [Bug 893956] Re: qemu-img bug with dynamic vhd
Quoting Stefan Hajnoczi (893...@bugs.launchpad.net): On Tue, Sep 4, 2012 at 4:00 PM, Serge Hallyn 893...@bugs.launchpad.net wrote: Though that commit and the comments were about 127G images. HIs is only 27G. The 127 GB limit applies to the virtual disk size, not to the size of the image file itself. I thought he had converted a roughly 26GB physical disk, assumed (perhaps wrongly) that was the virtual disk size. Also, 'qemu-img info' is also showing the error, which shows that this is not being done on vpc_create(). I'm not sure what you mean. Your commit (efc8243d00) added an -EFBIG return to vcp_open(), not vpc_create(). This will affect qemu-img info. Oops, good point. Thanks. We need the header/footer contents to be sure what's going on here. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/893956 Title: qemu-img bug with dynamic vhd Status in QEMU: New Bug description: Hye, i found a problem with qemu-img when trying to get info of a dynamic vhd. I made imgae of my 60GB computer hard drive with disk2vhd. The dynamic vhd is 21gb size. With 1.0-rc3 version : running command: qemu-img info 60_GB.VHD qemu-img: Could not open '60_GB.VHD' : File too large 0.14.1 version give me wrong information : image: 60_GB.VHD file format: vpc virtual size: 127G (13683600 bytes) disk size: 21G Thanks for reply. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/893956/+subscriptions
Re: [Qemu-devel] [PATCH 2/7] block: Framework for reopening files safely
Am 30.08.2012 20:47, schrieb Jeff Cody:
This is based heavily on Supriya Kannery's bdrv_reopen()
patch series.
This provides a transactional method to reopen multiple
images files safely.
Image files are queue for reopen via bdrv_reopen_queue(), and the
reopen occurs when bdrv_reopen_multiple() is called. Changes are
staged in bdrv_reopen_prepare() and in the equivalent driver level
functions. If any of the staged images fails a prepare, then all
of the images left untouched, and the staged changes for each image
abandoned.
Signed-off-by: Jeff Cody jc...@redhat.com
+/*
+ * Reopen multiple BlockDriverStates atomically transactionally.
+ *
+ * The queue passed in (bs_queue) must have been built up previous
+ * via bdrv_reopen_queue().
+ *
+ * Reopens all BDS specified in the queue, with the appropriate
+ * flags. All devices are prepared for reopen, and failure of any
+ * device will cause all device changes to be abandonded, and intermediate
+ * data cleaned up.
+ *
+ * If all devices prepare successfully, then the changes are committed
+ * to all devices.
+ *
+ */
+int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp)
+{
+int ret = -1;
+BlockReopenQueueEntry *bs_entry;
+Error *local_err = NULL;
+
+assert(bs_queue != NULL);
+
+bdrv_drain_all();
+
+QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {
+if (bdrv_reopen_prepare(bs_entry-state, local_err)) {
+error_propagate(errp, local_err);
+goto cleanup;
+}
+bs_entry-prepared = true;
+}
+
+/* If we reach this point, we have success and just need to apply the
+ * changes
+ */
+QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {
+bdrv_reopen_commit(bs_entry-state);
+}
+
+ret = 0;
+
+cleanup:
+QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {
+if (ret bs_entry-prepared) {
+bdrv_reopen_abort(bs_entry-state);
+}
+g_free(bs_entry-state);
+g_free(bs_entry);
+}
Without QSIMPLEQ_FOREACH_SAFE, isn't this a use after free?
+g_free(bs_queue);
+return ret;
+}
+
+
+/* Reopen a single BlockDriverState with the specified flags. */
+int bdrv_reopen(BlockDriverState *bs, int bdrv_flags, Error **errp)
+{
+int ret = -1;
+Error *local_err = NULL;
+BlockReopenQueue *queue = bdrv_reopen_queue(NULL, bs, bdrv_flags);
+
+ret = bdrv_reopen_multiple(queue, local_err);
+if (local_err != NULL) {
+error_propagate(errp, local_err);
+}
+return ret;
+}
+
+
+/*
+ * Prepares a BlockDriverState for reopen. All changes are staged in the
+ * 'reopen_state' field of the BlockDriverState, which must be NULL when
+ * entering (all previous reopens must have completed for the BDS).
+ *
+ * bs is the BlockDriverState to reopen
+ * flags are the new open flags
+ *
+ * Returns 0 on success, non-zero on error. On error errp will be set
+ * as well.
+ *
+ * On failure, bdrv_reopen_abort() will be called to clean up any data.
+ * It is the responsibility of the caller to then call the abort() or
+ * commit() for any other BDS that have been left in a prepare() state
+ *
+ */
+int bdrv_reopen_prepare(BDRVReopenState *reopen_state, Error **errp)
+{
+int ret = -1;
+Error *local_err = NULL;
+BlockDriver *drv;
+
+assert(reopen_state != NULL);
+assert(reopen_state-bs-drv != NULL);
+drv = reopen_state-bs-drv;
+
+/* if we are to stay read-only, do not allow permission change
+ * to r/w */
+if (reopen_state-bs-keep_read_only
Just for completeness, we decided to use the flag here instead of
keep_read_only.
+reopen_state-flags BDRV_O_RDWR) {
+error_set(errp, QERR_DEVICE_IS_READ_ONLY,
+ reopen_state-bs-device_name);
+goto error;
+}
+
+
+ret = bdrv_flush(reopen_state-bs);
+if (ret) {
+error_set(errp, QERR_IO_ERROR);
+goto error;
+}
This throws the error code away. Bad. We should probably change
QERR_IO_ERROR so that you can include strerror(-ret).
+
+if (drv-bdrv_reopen_prepare) {
+ret = drv-bdrv_reopen_prepare(reopen_state, local_err);
+if (ret) {
+if (local_err != NULL) {
+error_propagate(errp, local_err);
+} else {
+error_set(errp, QERR_OPEN_FILE_FAILED,
+ reopen_state-bs-filename);
+}
+goto error;
+}
+} else {
+/* It is currently mandatory to have a bdrv_reopen_prepare()
+ * handler for each supported drv. */
+error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
+ drv-format_name, reopen_state-bs-device_name,
+ reopening of file);
+ret = -1;
+goto error;
+}
+
+return 0;
+
+error:
+bdrv_reopen_abort(reopen_state);
This is
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On 09/05/2012 12:00 AM, Anthony Liguori wrote: Why? The way this is being submitted I don't see why we should treat Jan's patch any different from a patch by IBM or Samsung where we've asked folks to fix the license to comply with what I thought was our new policy (it does not even contain a from-x-on-GPLv2+ notice). Asking is one thing. Requiring is another. I would prefer that people submitted GPLv2+, but I don't think it should be a hard requirement. It means, among other things, that we cannot accept most code that originates from the Linux kernel. We could extend this to require unless there is a reason to grant an exception if we wanted to (not saying I know whether we want to or not). -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On Wed, Sep 05, 2012 at 06:26:54PM +0300, Avi Kivity wrote: On 09/05/2012 12:00 AM, Anthony Liguori wrote: Why? The way this is being submitted I don't see why we should treat Jan's patch any different from a patch by IBM or Samsung where we've asked folks to fix the license to comply with what I thought was our new policy (it does not even contain a from-x-on-GPLv2+ notice). Asking is one thing. Requiring is another. I would prefer that people submitted GPLv2+, but I don't think it should be a hard requirement. It means, among other things, that we cannot accept most code that originates from the Linux kernel. We could extend this to require unless there is a reason to grant an exception if we wanted to (not saying I know whether we want to or not). Would be nice to add a clarification in the header: people tend to copy boilerplate around. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 3/7] block: raw-posix image file reopen
Am 30.08.2012 20:47, schrieb Jeff Cody:
This is derived from the Supriya Kannery's reopen patches.
This contains the raw-posix driver changes for the bdrv_reopen_*
functions. All changes are staged into a temporary scratch buffer
during the prepare() stage, and copied over to the live structure
during commit(). Upon abort(), all changes are abandoned, and the
live structures are unmodified.
The _prepare() will create an extra fd - either by means of a dup,
if possible, or opening a new fd if not (for instance, access
control changes). Upon _commit(), the original fd is closed and
the new fd is used. Upon _abort(), the duplicate/new fd is closed.
Signed-off-by: Jeff Cody jc...@redhat.com
---
block/raw-posix.c | 153
+-
1 file changed, 139 insertions(+), 14 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 6be20b1..48086d7 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -140,6 +140,15 @@ typedef struct BDRVRawState {
#endif
} BDRVRawState;
+typedef struct BDRVRawReopenState {
+BDRVReopenState reopen_state;
+int fd;
+int open_flags;
+uint8_t *aligned_buf;
+unsigned aligned_buf_size;
+BDRVRawState *stash_s;
+} BDRVRawReopenState;
+
static int fd_open(BlockDriverState *bs);
static int64_t raw_getlength(BlockDriverState *bs);
@@ -185,6 +194,28 @@ static int raw_normalize_devicepath(const char
**filename)
}
#endif
+static void raw_parse_flags(int bdrv_flags, int *open_flags)
+{
+assert(open_flags != NULL);
+
+*open_flags |= O_BINARY;
+*open_flags = ~O_ACCMODE;
+if (bdrv_flags BDRV_O_RDWR) {
+*open_flags |= O_RDWR;
+} else {
+*open_flags |= O_RDONLY;
+}
+
+/* Use O_DSYNC for write-through caching, no flags for write-back
caching,
+ * and O_DIRECT for no caching. */
+if ((bdrv_flags BDRV_O_NOCACHE)) {
+*open_flags |= O_DIRECT;
+}
+if (!(bdrv_flags BDRV_O_CACHE_WB)) {
+*open_flags |= O_DSYNC;
+}
+}
The code motion would ideally be a separate patch.
+
static int raw_open_common(BlockDriverState *bs, const char *filename,
int bdrv_flags, int open_flags)
{
@@ -196,20 +227,8 @@ static int raw_open_common(BlockDriverState *bs, const
char *filename,
return ret;
}
-s-open_flags = open_flags | O_BINARY;
-s-open_flags = ~O_ACCMODE;
-if (bdrv_flags BDRV_O_RDWR) {
-s-open_flags |= O_RDWR;
-} else {
-s-open_flags |= O_RDONLY;
-}
-
-/* Use O_DSYNC for write-through caching, no flags for write-back
caching,
- * and O_DIRECT for no caching. */
-if ((bdrv_flags BDRV_O_NOCACHE))
-s-open_flags |= O_DIRECT;
-if (!(bdrv_flags BDRV_O_CACHE_WB))
-s-open_flags |= O_DSYNC;
+s-open_flags = open_flags;
+raw_parse_flags(bdrv_flags, s-open_flags);
s-fd = -1;
fd = qemu_open(filename, s-open_flags, 0644);
@@ -283,6 +302,109 @@ static int raw_open(BlockDriverState *bs, const char
*filename, int flags)
return raw_open_common(bs, filename, flags, 0);
}
+static int raw_reopen_prepare(BDRVReopenState *state, Error **errp)
+{
+BDRVRawState *s;
+BDRVRawReopenState *raw_s;
+int ret = 0;
+
+assert(state != NULL);
+assert(state-bs != NULL);
+
+s = state-bs-opaque;
+
+state-opaque = g_malloc0(sizeof(BDRVRawReopenState));
+raw_s = state-opaque;
+
+raw_parse_flags(state-flags, raw_s-open_flags);
+
+/*
+ * If we didn't have BDRV_O_NOCACHE set before, we may not have allocated
+ * aligned_buf
+ */
+if ((state-flags BDRV_O_NOCACHE)) {
+/*
+ * Allocate a buffer for read/modify/write cycles. Choose the size
+ * pessimistically as we don't know the block size yet.
+ */
+raw_s-aligned_buf_size = 32 * MAX_BLOCKSIZE;
+raw_s-aligned_buf = qemu_memalign(MAX_BLOCKSIZE,
+ raw_s-aligned_buf_size);
+
+if (raw_s-aligned_buf == NULL) {
+ret = -1;
+goto error;
+}
Even though it's pretty small, I think I would factor this out into a
small static helper to make sure it's kept in sync with raw_open_common().
+}
+
+int fcntl_flags = O_APPEND | O_ASYNC | O_NONBLOCK;
+#ifdef O_NOATIME
+fcntl_flags |= O_NOATIME;
+#endif
+if ((raw_s-open_flags ~fcntl_flags) == (s-open_flags
~fcntl_flags)) {
+/* dup the original fd */
+/* TODO: use qemu fcntl wrapper */
+raw_s-fd = fcntl(s-fd, F_DUPFD_CLOEXEC, 0);
+if (raw_s-fd == -1) {
+ret = -1;
+goto error;
+}
+ret = fcntl_setfl(raw_s-fd, raw_s-open_flags);
+} else {
+raw_s-fd = qemu_open(state-bs-filename, raw_s-open_flags, 0644);
+if (raw_s-fd
Re: [Qemu-devel] [PATCH 02/21] target-s390x: split FPU ops
On 09/04/2012 08:46 PM, Alexander Graf wrote: So that means your rewrite is based on this series and just fixes it up? Does that mean if I apply this patch, you will be all happy? It is not (yet) based on this series. But I will be happy if you apply it, since it's easier for me to rebase off master than find an external tree. r~
Re: [Qemu-devel] [PATCH] block: Don't forget to delete temporary file
Il 05/09/2012 15:26, riegama...@gmail.com ha scritto:
From: Dunrong Huang riegama...@gmail.com
The caller would not delete temporary file after failed get_tmp_filename().
Signed-off-by: Dunrong Huang riegama...@gmail.com
---
block.c | 6 +-
1 个文件被修改，插入 5 行(+)，删除 1 行(-)
diff --git a/block.c b/block.c
index 074987e..2bc9f75 100644
--- a/block.c
+++ b/block.c
@@ -433,7 +433,11 @@ int get_tmp_filename(char *filename, int size)
return -EOVERFLOW;
}
fd = mkstemp(filename);
-if (fd 0 || close(fd)) {
+if (fd 0) {
+return -errno;
+}
+if (close(fd) != 0) {
+unlink(filename);
return -errno;
}
return 0;
Not necessary, mkstemp will not create a file if it returns an error.
Paolo
Re: [Qemu-devel] [PATCH v3] block: output more error messages if failed to create temporary snapshot
Il 05/09/2012 15:24, riegama...@gmail.com ha scritto:
From: Dunrong Huang riegama...@gmail.com
If we failed to create temporary snapshot, the error message did not match
with the error, for example:
$ TMPDIR=/tmp/bad_path qemu-system-x86_64 -enable-kvm debian.qcow2 -snapshot
qemu-system-x86_64: -enable-kvm: could not open disk image
/home/mathslinux/Images/debian.qcow2: No such file or directory
Indeed, the file which cant be created is /tmp/bad_path/vl.xx, not
debian.qcow2. so the error message makes users feel confused.
Signed-off-by: Dunrong Huang riegama...@gmail.com
---
v1 - v2:
Output error message only if fd 0
v2 - v3:
Output error message in the caller of get_tmp_filename()
block.c | 2 ++
1 个文件被修改，插入 2 行(+)
diff --git a/block.c b/block.c
index 470bdcc..074987e 100644
--- a/block.c
+++ b/block.c
@@ -764,6 +764,8 @@ int bdrv_open(BlockDriverState *bs, const char *filename,
int flags,
ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename));
if (ret 0) {
+fprintf(stderr, Could not create temporary snapshot %s: %s\n,
+tmp_filename, strerror(errno));
return ret;
}
Looks good.
Paolo
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
Avi Kivity a...@redhat.com writes: On 09/05/2012 12:00 AM, Anthony Liguori wrote: Why? The way this is being submitted I don't see why we should treat Jan's patch any different from a patch by IBM or Samsung where we've asked folks to fix the license to comply with what I thought was our new policy (it does not even contain a from-x-on-GPLv2+ notice). Asking is one thing. Requiring is another. I would prefer that people submitted GPLv2+, but I don't think it should be a hard requirement. It means, among other things, that we cannot accept most code that originates from the Linux kernel. We could extend this to require unless there is a reason to grant an exception if we wanted to (not saying I know whether we want to or not). I don't want QEMU to be GPLv3. I don't like the terms of the GPLv3. I don't mind GPLv2+, if people want to share code from QEMU in GPLv3 projects, GPLv2+ enables that. But if new code is coming in and happens to be under GPLv2, that just means that the contribution cannot be used outside of QEMU in a GPLv3 project. That's fine and that's a decision for the submitter to make. Regards, Anthony Liguori -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH v4 5/5] i8259: fix dynamically masking slave IRQs with IMR register
On 2012-09-05 06:33, Matthew Ogilvie wrote:
According to later discussion, the main issue is actually the input
IRQ behavior on a high to low transition; hence the following fixes
both the test program and the Microport UNIX problem:
diff --git a/hw/i8259.c b/hw/i8259.c
index 6587666..c011787 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -143,22 +143,23 @@ static void pic_set_irq(void *opaque, int irq, int
level)
if (s-elcr mask) {
/* level triggered */
if (level) {
s-irr |= mask;
s-last_irr |= mask;
} else {
s-irr = ~mask;
s-last_irr = ~mask;
}
} else {
/* edge triggered */
if (level) {
if ((s-last_irr mask) == 0) {
s-irr |= mask;
}
s-last_irr |= mask;
} else {
+s-irr = ~mask;
s-last_irr = ~mask;
}
}
pic_update_irq(s);
}
Perhaps it would be worth it to swap around the ifs a little bit
to avoid the (!level) duplication, and clarify that the only difference
is in the low to high transition?
Yes, refactoring would be welcome. But I would suggest to do this in two
patches, leaving the above change (+ changelog that explains the reason)
in its current clarity.
Hurray, no vmstate subsections needed! :)
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] sparc-dis: Remove redundant NULL check
Am 03.09.2012 22:23, schrieb Stefan Weil:
Report from smatch:
sparc-dis.c:2664 build_hash_table(14) info:
redundant null check on hash_buf calling free()
Signed-off-by: Stefan Weil s...@weilnetz.de
---
Coding style was not fixed.
- sw
sparc-dis.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sparc-dis.c b/sparc-dis.c
index cdd337a..ef28835 100644
--- a/sparc-dis.c
+++ b/sparc-dis.c
@@ -2660,8 +2660,7 @@ build_hash_table (const sparc_opcode **opcode_table,
memset (hash_table, 0, HASH_SIZE * sizeof (hash_table[0]));
memset (hash_count, 0, HASH_SIZE * sizeof (hash_count[0]));
- if (hash_buf != NULL)
-free (hash_buf);
+ free (hash_buf);
hash_buf = malloc (sizeof (* hash_buf) * num_opcodes);
for (i = num_opcodes - 1; i = 0; --i)
{
*-dis sounds like binutils - did upstream drop the if, too?
If not, then diverging for a non-issue does not seem necessary.
Having said that, it's amazing what fixes you come up with. Did you
describe the use of that tool in some posting or in the Wiki?
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] block: Don't forget to delete temporary file
Hi, thanks for you reply.
2012/9/5 Paolo Bonzini pbonz...@redhat.com:
Il 05/09/2012 15:26, riegama...@gmail.com ha scritto:
From: Dunrong Huang riegama...@gmail.com
The caller would not delete temporary file after failed get_tmp_filename().
Signed-off-by: Dunrong Huang riegama...@gmail.com
---
block.c | 6 +-
1 个文件被修改，插入 5 行(+)，删除 1 行(-)
diff --git a/block.c b/block.c
index 074987e..2bc9f75 100644
--- a/block.c
+++ b/block.c
@@ -433,7 +433,11 @@ int get_tmp_filename(char *filename, int size)
return -EOVERFLOW;
}
fd = mkstemp(filename);
-if (fd 0 || close(fd)) {
+if (fd 0) {
+return -errno;
+}
+if (close(fd) != 0) {
+unlink(filename);
return -errno;
}
return 0;
Not necessary, mkstemp will not create a file if it returns an error.
If we call mkstemp() successfully, but failed to close(fd),
in this case, the temporafy file will not be deleted even if QEMU exits.
Paolo
--
Best Regards,
Dunrong Huang
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On 09/05/2012 06:41 PM, Anthony Liguori wrote: Avi Kivity a...@redhat.com writes: On 09/05/2012 12:00 AM, Anthony Liguori wrote: Why? The way this is being submitted I don't see why we should treat Jan's patch any different from a patch by IBM or Samsung where we've asked folks to fix the license to comply with what I thought was our new policy (it does not even contain a from-x-on-GPLv2+ notice). Asking is one thing. Requiring is another. I would prefer that people submitted GPLv2+, but I don't think it should be a hard requirement. It means, among other things, that we cannot accept most code that originates from the Linux kernel. We could extend this to require unless there is a reason to grant an exception if we wanted to (not saying I know whether we want to or not). I don't want QEMU to be GPLv3. I don't like the terms of the GPLv3. I don't mind GPLv2+, if people want to share code from QEMU in GPLv3 projects, GPLv2+ enables that. But if new code is coming in and happens to be under GPLv2, that just means that the contribution cannot be used outside of QEMU in a GPLv3 project. That's fine and that's a decision for the submitter to make. Makes sense. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] block: Don't forget to delete temporary file
Paolo Bonzini pbonz...@redhat.com writes:
Il 05/09/2012 15:26, riegama...@gmail.com ha scritto:
From: Dunrong Huang riegama...@gmail.com
The caller would not delete temporary file after failed get_tmp_filename().
Signed-off-by: Dunrong Huang riegama...@gmail.com
---
block.c | 6 +-
1 个文件被修改，插入 5 行(+)，删除 1 行(-)
diff --git a/block.c b/block.c
index 074987e..2bc9f75 100644
--- a/block.c
+++ b/block.c
@@ -433,7 +433,11 @@ int get_tmp_filename(char *filename, int size)
return -EOVERFLOW;
}
fd = mkstemp(filename);
-if (fd 0 || close(fd)) {
+if (fd 0) {
+return -errno;
+}
+if (close(fd) != 0) {
+unlink(filename);
return -errno;
}
return 0;
Not necessary, mkstemp will not create a file if it returns an error.
Read the patch once more :)
Re: [Qemu-devel] [PATCH] block: Don't forget to delete temporary file
Il 05/09/2012 18:02, Markus Armbruster ha scritto:
Paolo Bonzini pbonz...@redhat.com writes:
Il 05/09/2012 15:26, riegama...@gmail.com ha scritto:
From: Dunrong Huang riegama...@gmail.com
The caller would not delete temporary file after failed get_tmp_filename().
Signed-off-by: Dunrong Huang riegama...@gmail.com
---
block.c | 6 +-
1 个文件被修改，插入 5 行(+)，删除 1 行(-)
diff --git a/block.c b/block.c
index 074987e..2bc9f75 100644
--- a/block.c
+++ b/block.c
@@ -433,7 +433,11 @@ int get_tmp_filename(char *filename, int size)
return -EOVERFLOW;
}
fd = mkstemp(filename);
-if (fd 0 || close(fd)) {
+if (fd 0) {
+return -errno;
+}
+if (close(fd) != 0) {
+unlink(filename);
return -errno;
}
return 0;
Not necessary, mkstemp will not create a file if it returns an error.
Read the patch once more :)
Oops. :)
I wondered about that check for close() errors, perhaps an error in
close could just be swallowed? Since we don't care about the content of
the file after close (it's empty), we also don't care about errors
closing it.
However, perhaps there were errors writing the directory entry...
Should any errors writing the directory entry be reported by open(),
i.e. by mkstemp()? If not, and the dcache entry is evicted, someone
could create a different file with the same name as ours. But then, not
even a successful close() guarantees that the data has reached the disk.
And finally, the whole get_tmp_filename is unsafe because there is a
race window between closing and reopening the file, if the directory is
writable and does not have the sticky bit.
So the patch is an improvement, but there is still something unpleasing
in this code...
Paolo
Re: [Qemu-devel] [PATCH] block: Don't forget to delete temporary file
On 09/05/2012 10:23 AM, Paolo Bonzini wrote: And finally, the whole get_tmp_filename is unsafe because there is a race window between closing and reopening the file, if the directory is writable and does not have the sticky bit. So the patch is an improvement, but there is still something unpleasing in this code... I absolutely agree that there is a nasty race here. If you aren't going to use the fd, then mktemp() is sufficient (and just as racy, but then you are at least honest that you don't care about the race); in all other situations, if you want a temporary file name but want to avoid a race, then it feels like you should be returning the fd from mkstemp() still open (or at a bare minimum, auditing ALL callers to make sure they only use the temporary name with O_CREAT|O_EXCL, and that they retry in a loop in case they lose the race, at which point they are reinventing the loop already done on their behalf by mkstemp()...). -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 2/7] block: Framework for reopening files safely
On 09/05/2012 11:09 AM, Kevin Wolf wrote:
Am 30.08.2012 20:47, schrieb Jeff Cody:
This is based heavily on Supriya Kannery's bdrv_reopen()
patch series.
This provides a transactional method to reopen multiple
images files safely.
Image files are queue for reopen via bdrv_reopen_queue(), and the
reopen occurs when bdrv_reopen_multiple() is called. Changes are
staged in bdrv_reopen_prepare() and in the equivalent driver level
functions. If any of the staged images fails a prepare, then all
of the images left untouched, and the staged changes for each image
abandoned.
Signed-off-by: Jeff Cody jc...@redhat.com
+/*
+ * Reopen multiple BlockDriverStates atomically transactionally.
+ *
+ * The queue passed in (bs_queue) must have been built up previous
+ * via bdrv_reopen_queue().
+ *
+ * Reopens all BDS specified in the queue, with the appropriate
+ * flags. All devices are prepared for reopen, and failure of any
+ * device will cause all device changes to be abandonded, and intermediate
+ * data cleaned up.
+ *
+ * If all devices prepare successfully, then the changes are committed
+ * to all devices.
+ *
+ */
+int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp)
+{
+int ret = -1;
+BlockReopenQueueEntry *bs_entry;
+Error *local_err = NULL;
+
+assert(bs_queue != NULL);
+
+bdrv_drain_all();
+
+QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {
+if (bdrv_reopen_prepare(bs_entry-state, local_err)) {
+error_propagate(errp, local_err);
+goto cleanup;
+}
+bs_entry-prepared = true;
+}
+
+/* If we reach this point, we have success and just need to apply the
+ * changes
+ */
+QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {
+bdrv_reopen_commit(bs_entry-state);
+}
+
+ret = 0;
+
+cleanup:
+QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {
+if (ret bs_entry-prepared) {
+bdrv_reopen_abort(bs_entry-state);
+}
+g_free(bs_entry-state);
+g_free(bs_entry);
+}
Without QSIMPLEQ_FOREACH_SAFE, isn't this a use after free?
Yes - that needs to be a QSIMPLEQ_FOREACH_SAFE.
+g_free(bs_queue);
+return ret;
+}
+
+
+/* Reopen a single BlockDriverState with the specified flags. */
+int bdrv_reopen(BlockDriverState *bs, int bdrv_flags, Error **errp)
+{
+int ret = -1;
+Error *local_err = NULL;
+BlockReopenQueue *queue = bdrv_reopen_queue(NULL, bs, bdrv_flags);
+
+ret = bdrv_reopen_multiple(queue, local_err);
+if (local_err != NULL) {
+error_propagate(errp, local_err);
+}
+return ret;
+}
+
+
+/*
+ * Prepares a BlockDriverState for reopen. All changes are staged in the
+ * 'reopen_state' field of the BlockDriverState, which must be NULL when
+ * entering (all previous reopens must have completed for the BDS).
+ *
+ * bs is the BlockDriverState to reopen
+ * flags are the new open flags
+ *
+ * Returns 0 on success, non-zero on error. On error errp will be set
+ * as well.
+ *
+ * On failure, bdrv_reopen_abort() will be called to clean up any data.
+ * It is the responsibility of the caller to then call the abort() or
+ * commit() for any other BDS that have been left in a prepare() state
+ *
+ */
+int bdrv_reopen_prepare(BDRVReopenState *reopen_state, Error **errp)
+{
+int ret = -1;
+Error *local_err = NULL;
+BlockDriver *drv;
+
+assert(reopen_state != NULL);
+assert(reopen_state-bs-drv != NULL);
+drv = reopen_state-bs-drv;
+
+/* if we are to stay read-only, do not allow permission change
+ * to r/w */
+if (reopen_state-bs-keep_read_only
Just for completeness, we decided to use the flag here instead of
keep_read_only.
+reopen_state-flags BDRV_O_RDWR) {
+error_set(errp, QERR_DEVICE_IS_READ_ONLY,
+ reopen_state-bs-device_name);
+goto error;
+}
+
+
+ret = bdrv_flush(reopen_state-bs);
+if (ret) {
+error_set(errp, QERR_IO_ERROR);
+goto error;
+}
This throws the error code away. Bad. We should probably change
QERR_IO_ERROR so that you can include strerror(-ret).
Or, I could use the new error_setg() here, and pass along all relevant
information.
+
+if (drv-bdrv_reopen_prepare) {
+ret = drv-bdrv_reopen_prepare(reopen_state, local_err);
+if (ret) {
+if (local_err != NULL) {
+error_propagate(errp, local_err);
+} else {
+error_set(errp, QERR_OPEN_FILE_FAILED,
+ reopen_state-bs-filename);
+}
+goto error;
+}
+} else {
+/* It is currently mandatory to have a bdrv_reopen_prepare()
+ * handler for each supported drv. */
+error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
+ drv-format_name,
[Qemu-devel] [ANNOUNCE] QEMU 1.2.0 release
Hi, On behalf of the QEMU Team, I'd like to announce the availability of the QEMU 1.2 release! http://wiki.qemu.org/download/qemu-1.2.0.tar.bz2 Even though this was the shortest release cycle in QEMU's history, it contains an impressive 1400 changesets from 180 unique authors. See the ChangeLog on the wiki for a full changelog: http://wiki.qemu.org/ChangeLog/1.2 Major features include: - The ability to pass fds for block devices enabling sVirt on NFS - arm: LPAE support for the Cortex-A15 CPU - arm: a new machine type: i.MX32 - A new dump-guest-memory command to produce ELF dumps of guests - pseries: support for PCI and IOMMU emulation - ppc: better support for device trees including a dumpdtb option - ppc: emulation of e5500 cores - prep: parallel port emulation - xen: support for PCI passthrough - kvm: MSI support for in-kernel APIC - vga: std-vga and QXL now support 16MB of VRAM - scsi: emulation of am53c974, dc390, and megasas controllers - usb: emulation of scsi controller - block: guest control of write-cache setting - block: default cache mode is now writeback - block: improved support for passthrough of SCSI tapes changers - achi: emulation of CD-ROM devices - block-stream: better support for sparse raw files - qcow2: lazy refcount mode for improved writethrough performance - qemu-img: check command can now repair qcow2/qed images - migration: support for large memory guests - migration: support for XBZRLE compression - vnc: threaded server enabled by default - qemu-ga: support for fstrim - openrisc emulation support - and much, much more... Deprecation announcements: - The 'cpudef' config file section will be removed in 1.3 - The -help output from QEMU will change in 1.3 I'd like to thank everyone who contributed to this release by submitting patches, testing out -rcs, or reporting bugs during the release process! Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 3/7] block: raw-posix image file reopen
On 09/05/2012 11:30 AM, Kevin Wolf wrote:
Am 30.08.2012 20:47, schrieb Jeff Cody:
This is derived from the Supriya Kannery's reopen patches.
This contains the raw-posix driver changes for the bdrv_reopen_*
functions. All changes are staged into a temporary scratch buffer
during the prepare() stage, and copied over to the live structure
during commit(). Upon abort(), all changes are abandoned, and the
live structures are unmodified.
The _prepare() will create an extra fd - either by means of a dup,
if possible, or opening a new fd if not (for instance, access
control changes). Upon _commit(), the original fd is closed and
the new fd is used. Upon _abort(), the duplicate/new fd is closed.
Signed-off-by: Jeff Cody jc...@redhat.com
---
block/raw-posix.c | 153
+-
1 file changed, 139 insertions(+), 14 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 6be20b1..48086d7 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -140,6 +140,15 @@ typedef struct BDRVRawState {
#endif
} BDRVRawState;
+typedef struct BDRVRawReopenState {
+BDRVReopenState reopen_state;
+int fd;
+int open_flags;
+uint8_t *aligned_buf;
+unsigned aligned_buf_size;
+BDRVRawState *stash_s;
+} BDRVRawReopenState;
+
static int fd_open(BlockDriverState *bs);
static int64_t raw_getlength(BlockDriverState *bs);
@@ -185,6 +194,28 @@ static int raw_normalize_devicepath(const char
**filename)
}
#endif
+static void raw_parse_flags(int bdrv_flags, int *open_flags)
+{
+assert(open_flags != NULL);
+
+*open_flags |= O_BINARY;
+*open_flags = ~O_ACCMODE;
+if (bdrv_flags BDRV_O_RDWR) {
+*open_flags |= O_RDWR;
+} else {
+*open_flags |= O_RDONLY;
+}
+
+/* Use O_DSYNC for write-through caching, no flags for write-back
caching,
+ * and O_DIRECT for no caching. */
+if ((bdrv_flags BDRV_O_NOCACHE)) {
+*open_flags |= O_DIRECT;
+}
+if (!(bdrv_flags BDRV_O_CACHE_WB)) {
+*open_flags |= O_DSYNC;
+}
+}
The code motion would ideally be a separate patch.
OK
+
static int raw_open_common(BlockDriverState *bs, const char *filename,
int bdrv_flags, int open_flags)
{
@@ -196,20 +227,8 @@ static int raw_open_common(BlockDriverState *bs, const
char *filename,
return ret;
}
-s-open_flags = open_flags | O_BINARY;
-s-open_flags = ~O_ACCMODE;
-if (bdrv_flags BDRV_O_RDWR) {
-s-open_flags |= O_RDWR;
-} else {
-s-open_flags |= O_RDONLY;
-}
-
-/* Use O_DSYNC for write-through caching, no flags for write-back
caching,
- * and O_DIRECT for no caching. */
-if ((bdrv_flags BDRV_O_NOCACHE))
-s-open_flags |= O_DIRECT;
-if (!(bdrv_flags BDRV_O_CACHE_WB))
-s-open_flags |= O_DSYNC;
+s-open_flags = open_flags;
+raw_parse_flags(bdrv_flags, s-open_flags);
s-fd = -1;
fd = qemu_open(filename, s-open_flags, 0644);
@@ -283,6 +302,109 @@ static int raw_open(BlockDriverState *bs, const char
*filename, int flags)
return raw_open_common(bs, filename, flags, 0);
}
+static int raw_reopen_prepare(BDRVReopenState *state, Error **errp)
+{
+BDRVRawState *s;
+BDRVRawReopenState *raw_s;
+int ret = 0;
+
+assert(state != NULL);
+assert(state-bs != NULL);
+
+s = state-bs-opaque;
+
+state-opaque = g_malloc0(sizeof(BDRVRawReopenState));
+raw_s = state-opaque;
+
+raw_parse_flags(state-flags, raw_s-open_flags);
+
+/*
+ * If we didn't have BDRV_O_NOCACHE set before, we may not have
allocated
+ * aligned_buf
+ */
+if ((state-flags BDRV_O_NOCACHE)) {
+/*
+ * Allocate a buffer for read/modify/write cycles. Choose the size
+ * pessimistically as we don't know the block size yet.
+ */
+raw_s-aligned_buf_size = 32 * MAX_BLOCKSIZE;
+raw_s-aligned_buf = qemu_memalign(MAX_BLOCKSIZE,
+ raw_s-aligned_buf_size);
+
+if (raw_s-aligned_buf == NULL) {
+ret = -1;
+goto error;
+}
Even though it's pretty small, I think I would factor this out into a
small static helper to make sure it's kept in sync with raw_open_common().
OK, and like your suggestion above, I'll put that in a separate code
motion patch.
+}
+
+int fcntl_flags = O_APPEND | O_ASYNC | O_NONBLOCK;
+#ifdef O_NOATIME
+fcntl_flags |= O_NOATIME;
+#endif
+if ((raw_s-open_flags ~fcntl_flags) == (s-open_flags
~fcntl_flags)) {
+/* dup the original fd */
+/* TODO: use qemu fcntl wrapper */
+raw_s-fd = fcntl(s-fd, F_DUPFD_CLOEXEC, 0);
+if (raw_s-fd == -1) {
+ret = -1;
+goto error;
+}
+ret =
[Qemu-devel] [ANNOUNCE] 1.3 development is now open!
I've updated the release plan on the wiki: http://wiki.qemu.org/Planning/1.3 No surprises, same format/timeline as 1.2. Happy hacking! Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH] sparc-dis: Remove redundant NULL check
On Wed, Sep 5, 2012 at 4:51 PM, Andreas Färber afaer...@suse.de wrote:
Am 03.09.2012 22:23, schrieb Stefan Weil:
Report from smatch:
sparc-dis.c:2664 build_hash_table(14) info:
redundant null check on hash_buf calling free()
Signed-off-by: Stefan Weil s...@weilnetz.de
---
Coding style was not fixed.
- sw
sparc-dis.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sparc-dis.c b/sparc-dis.c
index cdd337a..ef28835 100644
--- a/sparc-dis.c
+++ b/sparc-dis.c
@@ -2660,8 +2660,7 @@ build_hash_table (const sparc_opcode **opcode_table,
memset (hash_table, 0, HASH_SIZE * sizeof (hash_table[0]));
memset (hash_count, 0, HASH_SIZE * sizeof (hash_count[0]));
- if (hash_buf != NULL)
-free (hash_buf);
+ free (hash_buf);
hash_buf = malloc (sizeof (* hash_buf) * num_opcodes);
for (i = num_opcodes - 1; i = 0; --i)
{
*-dis sounds like binutils - did upstream drop the if, too?
If not, then diverging for a non-issue does not seem necessary.
Ah, good point. I don't think we should apply this patch :P.
Dropping the patch from qemu-trivial until this discussion finishes.
Stefan
[Qemu-devel] Virtual Machine Extension Instructions In QEMU
I would like to know how well is the Intel VMX and AMD SVM supported in QEMU ? Xin
Re: [Qemu-devel] [PATCH] sparc-dis: Remove redundant NULL check
Am 05.09.2012 18:55, schrieb Stefan Hajnoczi:
On Wed, Sep 5, 2012 at 4:51 PM, Andreas Färberafaer...@suse.de wrote:
Am 03.09.2012 22:23, schrieb Stefan Weil:
Report from smatch:
sparc-dis.c:2664 build_hash_table(14) info:
redundant null check on hash_buf calling free()
Signed-off-by: Stefan Weils...@weilnetz.de
---
Coding style was not fixed.
- sw
sparc-dis.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sparc-dis.c b/sparc-dis.c
index cdd337a..ef28835 100644
--- a/sparc-dis.c
+++ b/sparc-dis.c
@@ -2660,8 +2660,7 @@ build_hash_table (const sparc_opcode **opcode_table,
memset (hash_table, 0, HASH_SIZE * sizeof (hash_table[0]));
memset (hash_count, 0, HASH_SIZE * sizeof (hash_count[0]));
- if (hash_buf != NULL)
-free (hash_buf);
+ free (hash_buf);
hash_buf = malloc (sizeof (* hash_buf) * num_opcodes);
for (i = num_opcodes - 1; i= 0; --i)
{
*-dis sounds like binutils - did upstream drop the if, too?
If not, then diverging for a non-issue does not seem necessary.
Ah, good point. I don't think we should apply this patch :P.
Dropping the patch from qemu-trivial until this discussion finishes.
Stefan
AFAIK, binutils moved to GPL 3. Therefore I don't expect that
QEMU will update to upstream in the next years.
We'll have to maintain the code which we have.
Try git log *-dis.c or gitk *-dis.c: there are already lots
of more trivial changes which got applied to the disassembler files.
= The patch should be applied.
Regards,
Stefan
Re: [Qemu-devel] [PATCH] sparc-dis: Remove redundant NULL check
Am 05.09.2012 19:15, schrieb Stefan Weil: AFAIK, binutils moved to GPL 3. Therefore I don't expect that QEMU will update to upstream in the next years. We'll have to maintain the code which we have. Try git log *-dis.c or gitk *-dis.c: there are already lots of more trivial changes which got applied to the disassembler files. = The patch should be applied. Regards, Stefan Here is some additional information: binutils switched from GPL 2 to GPL 3 with version 2.18: Changes in 2.18: * The binutils sources are now released under version 3 of the GNU General Public License. sparc-dis.c is already based on 2.17, so we won't get anything newer. Even the latest version still uses the redundant NULL check, so I can send my patch to the binutils maintainers, too. Regards, Stefan
Re: [Qemu-devel] [PATCH] sparc-dis: Remove redundant NULL check
Am 05.09.2012 17:51, schrieb Andreas Färber:
Am 03.09.2012 22:23, schrieb Stefan Weil:
Report from smatch:
sparc-dis.c:2664 build_hash_table(14) info:
redundant null check on hash_buf calling free()
Signed-off-by: Stefan Weils...@weilnetz.de
---
Coding style was not fixed.
- sw
sparc-dis.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sparc-dis.c b/sparc-dis.c
index cdd337a..ef28835 100644
--- a/sparc-dis.c
+++ b/sparc-dis.c
@@ -2660,8 +2660,7 @@ build_hash_table (const sparc_opcode **opcode_table,
memset (hash_table, 0, HASH_SIZE * sizeof (hash_table[0]));
memset (hash_count, 0, HASH_SIZE * sizeof (hash_count[0]));
- if (hash_buf != NULL)
-free (hash_buf);
+ free (hash_buf);
hash_buf = malloc (sizeof (* hash_buf) * num_opcodes);
for (i = num_opcodes - 1; i= 0; --i)
{
*-dis sounds like binutils - did upstream drop the if, too?
If not, then diverging for a non-issue does not seem necessary.
Having said that, it's amazing what fixes you come up with. Did you
describe the use of that tool in some posting or in the Wiki?
I just added some initial text to http://wiki.qemu.org/Testing.
Regards,
Andreas
Re: [Qemu-devel] Help needed to run Exynos 4210 linux kernel on qemu?
Hello Igor, Thanks for you reply and your time. JC On 09/05/2012 02:38 PM, Igor Mitsyanko wrote: On 09/04/2012 02:13 AM, Jean-Christophe DUBOIS wrote: Hello, Jean! I've just tried to do the same thing you're trying to do, it works for me if I remove earlyprintk from append. Yes, I found out about this in the mean time. Probably this is due to the fact that kernel expects uart0 to be configured by u-boot prior to kernel start execution. Interestingly, if you change default exynos4_defconfig CONFIG_DEBUG_S3C_UART0 to, for example, CONFIG_DEBUG_S3C_UART2 in linux kernel configuration, everything starting to work fine even with earlyprintk. Seems strange. It would be nice if earlyprintk was not hanging though (is it possible to work around the actual LSP expectation?). But using ramdisks is always a pain for me, I would suggest you two different approaches: Ramdisk is fine for what I need to do at this time. 1) The most convenient way is to use emulated SD card for rootfs storage. There is no SD card support for exynos4210 in current QEMU master, but you can apply SD host controller patches I attached to this email to get this support. These patches already have been thoroughly reviewed in qemu-devel mailing list and hopefully would be applied to QEMU master soon enough. If you want to use SD card instead of RAMDISK, apply these patches to QEMU master with git am, and enable MMC/SD support in linux kernel configuration (it is disabled by default in exynos4_defconfig). You need to check CONFIG_MMC, CONFIG_MMC_SDHC, CONFIG_MMC_SDHCI_S3C and ONFIG_MMC_SDHCI_S3C_DMA. Then you can launch QEMU with -sd /path/to/your/rootfs and, for example, append line -append root=/dev/mmcblk0 rootfstype=ext4 rw rootwait. Thanks. That seems interesting but I don't need it right now. I can wait for this patch to hit mainline. 2) If you want to work only with what current QEMU master has, you can use network filesystem as rootfs, but only for smdkc210 board (nuri board doesn't have any net devices). You're gonna have to check these options in Linux configuration: CONFIG_NET, CONFIG_PACKET, CONFIG_INET, CONFIG_IP_PNP, CONFIG_IP_PNP_DHCP, CONFIG_NETDEVICES, CONFIG_SMSC911X, CONFIG_NFS_FS, CONFIG_ROOT_NFS. Then, if, for example, you have directory /mnt/nfsroot exported by NFS server on your host machine, you can launch QEMU guest with following command line: qemu-system-arm -kernel ./zImage -M smdkc210 -append console=ttySAC0,115200n8 ip=dhcp root=/dev/nfs nfsroot=10.0.2.2:/mnt/nfsroot/ rw -serial stdio Unfortunately, network will have to wait too. But it will come in a second step. Also, maybe you noticed, graphic support is disabled in exynos4_defconfig by default. To enable it, check these options in Linux configuration: CONFIG_DRM, CONFIG_DRM_EXYNOS, CONFIG_DRM_EXYNOS_DMABUF, CONFIG_DRM_EXYNOS_FIMD. You can also check CONFIG_LOGO to get a fancy penguins during guest VM startup. And if you need to start X server on your guest VM, you also need to enable UNIX domain sockets in linux kernel CONFIG_UNIX. Thanks. I'll look at this later. No graphic is needed for now. A serial console is all I need.
Re: [Qemu-devel] [PATCH 09/19] sockets: Chardev open error reporting, sockets part
On Wed, 5 Sep 2012 10:19:22 +0800
Amos Kong kongjian...@gmail.com wrote:
On Thu, Aug 30, 2012 at 12:04 AM, Amos Kong kongjian...@gmail.com wrote:
On Wed, Aug 29, 2012 at 11:15 PM, Amos Kong kongjian...@gmail.com wrote:
On Thu, Feb 23, 2012 at 4:15 PM, Markus Armbruster arm...@redhat.com
wrote:
Anthony Liguori anth...@codemonkey.ws writes:
On 02/15/2012 07:33 AM, Markus Armbruster wrote:
Anthony Liguorianth...@codemonkey.ws writes:
On 02/14/2012 11:24 AM, Markus Armbruster wrote:
Markus Armbrusterarm...@redhat.com writes:
Anthony Liguorialigu...@us.ibm.com writes:
[Anthony asking for error_set() instead of error_report()...]
Basically, same thing here and the remaining functions. Let's
not
introduce additional uses of error_report().
That said, I imagine you don't want to introduce a bunch of error
types for these different things and that's probably not
productive
anyway.
[...]
So let's compromise and introduce a generic QERR_INTERNAL_ERROR
that
takes a single human readable string as an argument. We can
have a
wrapper for it that also records location information in the
error
object.
This series goes from stderr to error_report(). That's a
relatively
simple step, which makes it relatively easy to review. I'm afraid
moving all the way to error.h in one step wouldn't be as easy.
Kevin
suggests to do it in a follow-up series, and I agree.
The trouble I have is not about doing things incrementally, but
rather
touching a lot of code incrementally. Most of the code you touch
could be done incrementally with error_set().
For instance, you could touch inet_listen_opts() and just add an
Error
** as the last argument. You can change all callers to simply do:
Error *err = NULL;
...
inet_listen_opts(...,err);
if (err) {
error_report_err(err);
return -1;
}
And it's not really all that different from the series as it stands
today. I agree that aggressively refactoring error propagation is
probably not necessary as a first step, but if we're going to touch
a
lot of code, we should do it in a way that we don't have to
immediately touch it again next.
Well, the series adds 47 calls of error_report() to five files out of
1850.
Can you point to an existing conversion from error_report() to
error.h,
to give us an idea how it's supposed to be done?
Ping?
Sorry, I mentally responded bug neglected to actually respond.
All of the QMP work that Luiz is doing effectively does this so
there
are ample examples right now. The change command is probably a good
place to start.
Thanks.
Unfortunately, I'm out of time on this one, so if you're unwilling to
accept this admittedly incremental improvement without substantial
rework, I'll have to let it rot in peace.
We might want a QMP commands to add character devices some day.
Perhaps
the person doing it will still be able to find these patches, and get
some use out of them.
Patches 01-08,14 don't add error_report() calls. What about
committing
them? The commit messages would need to be reworded not to promise
goodies from the other patches, though.
I'm sorry to hear that you can't continue working on this.
Can't be helped.
I want to continue working on this work (patch 9~13,15~19).
FYI, URL of the old thread:
http://lists.nongnu.org/archive/html/qemu-devel/2012-02/msg00714.html
Makus used error_report() to output the rich/meaningful error message
to monitor,
but Anthony prefers to use error_set(), right?
I would like to introduce a generic QERR_INTERNAL_ERROR as Anthony
said to replace error_report().
error_report() can be used many times/places, we might see many error
in monitor.
but error_set() can only set one kind of error when internal error
occurs, and only
one error will be printed into monitor.
output final/important error by error_set()/error_report_err(), and
output other error to stdio?
---
There are some 'GENERIC ERROR CLASS' in qerror.h, which are not used
very frequently, we can convert them to 'QERR_INTERNAL_ERROR'.
Or convert all 'GENERIC ERROR CLASS' to 'QERR_INTERNAL_ERROR',
and use a single human readable string?
Please help to review below RFC patch, thanks.
Anthony, Luiz, other
any comments?
archive of this patch:
http://lists.nongnu.org/archive/html/qemu-devel/2012-08/msg04927.html
That thread is too old, that work should be re-done on top of the new
error format.
However, we have bug 603266 for that and it has been assigned to me. And
I have started working on it already...
From 4b8200ce662dd375819fd24cb932e70131ce0bd3 Mon Sep 17 00:00:00 2001
From: Amos Kong
[Qemu-devel] [PULL 00/19]: QMP queue
Let's get the ball rolling for QMP in 1.3 :) This pull request contains the send-key command conversion, screendump qapi conversion and a few fixes. The changes (since f45ddd14209a4d1b95a4096d50a561b7f6270118) are available in the following repository: git://repo.or.cz/qemu/qmp-unstable.git queue/qmp Amos Kong (7): fix doc of using raw values with sendkey monitor: rename keyname '' to 'less' hmp: rename arguments qapi: generate list struct and visit_list for enum qapi: add the QKeyCode enum monitor: move key_defs[] table and introduce two help functions qapi: convert sendkey Daniel P. Berrange (1): Add support for pretty-printing response in qmp-shell Luiz Capitulino (9): error: add error_setg() console: vga_hw_screen_dump_ptr: take Error argument qapi: convert screendump vga: ppm_save(): add error handling omap_lcdc: rename ppm_save() to omap_ppm_save() omap_lcdc: omap_ppm_save(): add error handling g364fb: g364fb_screen_dump(): add error handling tcx: tcx24_screen_dump(): add error handling tcx: tcx_screen_dump(): add error handling Stefan Weil (2): qapi: Fix potential NULL pointer segfault json-parser: Fix potential NULL pointer segfault QMP/qmp-shell | 46 ++--- console.c | 7 +- console.h | 10 +- error.h | 6 ++ hmp-commands.hx | 13 ++- hmp.c | 64 + hmp.h | 2 + hw/blizzard.c | 4 +- hw/g364fb.c | 55 --- hw/omap_lcdc.c| 66 + hw/qxl.c | 7 +- hw/tcx.c | 97 +++ hw/vga.c | 38 ++-- hw/vga_int.h | 3 +- hw/vmware_vga.c | 7 +- input.c | 251 ++ monitor.c | 251 +- qapi-schema.json | 59 qmp-commands.hx | 33 ++- qobject.h | 2 +- scripts/qapi-types.py | 16 +++- scripts/qapi-visit.py | 16 +++- 22 files changed, 704 insertions(+), 349 deletions(-)
Re: [Qemu-devel] [PATCH] target-i386: Allow changing of Hypervisor CPUIDs.
On Thu, Aug 30, 2012 at 03:20:35PM -0400, Don Slutz wrote:
This is primarily done so that the guest will think it is running
under vmware when hypervisor=vmware is specified as a property of a
cpu.
Also allow this to work in accel=tcg mode.
The new cpu properties hyper_level, hyper_extra, hyper_extra_a, and
hyper_extra_b can be used to further adjust what the guest sees.
Signed-off-by: Don Slutz d...@cloudswitch.com
For what purpose?
Is the VMWare interface documented somewhere?
---
target-i386/cpu.c | 178
+
target-i386/cpu.h |9 +++
target-i386/kvm.c | 33 --
3 files changed, 214 insertions(+), 6 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index f3cac49..a444b95 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -26,6 +26,7 @@
#include qemu-option.h
#include qemu-config.h
+#include qemu-timer.h
#include qapi/qapi-visit-core.h
#include arch_init.h
@@ -244,6 +245,15 @@ typedef struct x86_def_t {
uint32_t xlevel2;
/* The feature bits on CPUID[EAX=7,ECX=0].EBX */
uint32_t cpuid_7_0_ebx_features;
+/* Hypervisor CPUIDs */
+uint32_t cpuid_hv_level;
+uint32_t cpuid_hv_vendor1;
+uint32_t cpuid_hv_vendor2;
+uint32_t cpuid_hv_vendor3;
+/* VMware extra data */
+uint32_t cpuid_hv_extra;
+uint32_t cpuid_hv_extra_a;
+uint32_t cpuid_hv_extra_b;
} x86_def_t;
#define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
@@ -860,6 +870,18 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor
*v, void *opaque,
cpu-env.tsc_khz = value / 1000;
}
+static void x86_cpuid_set_hv(x86_def_t *x86_cpu_def, uint32_t level,
+ const char *who)
+{
+uint32_t signature[3];
+
+memcpy(signature, who, 12);
+x86_cpu_def-cpuid_hv_level = level;
+x86_cpu_def-cpuid_hv_vendor1 = signature[0];
+x86_cpu_def-cpuid_hv_vendor2 = signature[1];
+x86_cpu_def-cpuid_hv_vendor3 = signature[2];
+}
+
static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char
*cpu_model)
{
unsigned int i;
@@ -867,6 +889,10 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def,
const char *cpu_model)
char *s = g_strdup(cpu_model);
char *featurestr, *name = strtok(s, ,);
+bool hyperv_enabled = false;
+bool hv_enabled = false;
+long hyper_level = -1;
+long hyper_extra = -1;
/* Features to be added*/
uint32_t plus_features = 0, plus_ext_features = 0;
uint32_t plus_ext2_features = 0, plus_ext3_features = 0;
@@ -993,12 +1019,84 @@ static int cpu_x86_find_by_name(x86_def_t
*x86_cpu_def, const char *cpu_model)
x86_cpu_def-tsc_khz = tsc_freq / 1000;
} else if (!strcmp(featurestr, hv_spinlocks)) {
char *err;
+
+if (hv_enabled) {
+fprintf(stderr,
+Only one of hypervisor= or hv_* can be used at
one time.\n);
+goto error;
+}
numvalue = strtoul(val, err, 0);
if (!*val || *err) {
fprintf(stderr, bad numerical value %s\n, val);
goto error;
}
+hyperv_enabled = true;
hyperv_set_spinlock_retries(numvalue);
+} else if (!strcmp(featurestr, hyper_level)) {
+char *err;
+long longvalue = strtol(val, err, 0);
+
+if (!*val || *err) {
+fprintf(stderr, bad numerical value for
hyper_level=%s\n,
+val);
+goto error;
+}
+hyper_level = longvalue;
+} else if (!strcmp(featurestr, hyper_extra)) {
+char *err;
+long longvalue = strtol(val, err, 0);
+
+if (!*val || *err) {
+fprintf(stderr, bad numerical value for
hyper_extra=%s\n,
+val);
+goto error;
+}
+hyper_extra = longvalue;
+} else if (!strcmp(featurestr, hyper_extra_a)) {
+char *err;
+
+numvalue = strtoul(val, err, 0);
+if (!*val || *err) {
+fprintf(stderr,
+bad numerical value for hyper_extra_a=%s\n,
+val);
+goto error;
+}
+x86_cpu_def-cpuid_hv_extra_a = (uint32_t)numvalue;
+} else if (!strcmp(featurestr, hyper_extra_b)) {
+char *err;
+
+numvalue = strtoul(val, err, 0);
+if (!*val || *err) {
+fprintf(stderr,
+bad numerical value for
[Qemu-devel] [PATCH] socket: don't attempt to reconnect a TCP socket in server mode
Commit c3767ed0eb5d0bb25fe409ae5dec06e3411ff1b6 introduced a possible SEGV when
using a socket chardev with server=on because it assumes that all TCP sockets
are in client mode.
This patch adds a check to only reconnect when in client mode.
Cc: Lei Li li...@linux.vnet.ibm.com
Reported-by: Michael Roth mdr...@linux.vnet.ibm.com
Signed-off-by: Anthony Liguori aligu...@us.ibm.com
---
qemu-char.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 398baf1..767da93 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2148,10 +2148,12 @@ static int tcp_chr_write(CharDriverState *chr, const
uint8_t *buf, int len)
TCPCharDriver *s = chr-opaque;
if (s-connected) {
return send_all(s-fd, buf, len);
-} else {
+} else if (s-listen_fd == -1) {
/* (Re-)connect for unconnected writing */
tcp_chr_connect(chr);
return 0;
+} else {
+return len;
}
}
--
1.7.5.4
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On Wed, Sep 5, 2012 at 3:41 PM, Anthony Liguori anth...@codemonkey.ws wrote: Avi Kivity a...@redhat.com writes: On 09/05/2012 12:00 AM, Anthony Liguori wrote: Why? The way this is being submitted I don't see why we should treat Jan's patch any different from a patch by IBM or Samsung where we've asked folks to fix the license to comply with what I thought was our new policy (it does not even contain a from-x-on-GPLv2+ notice). Asking is one thing. Requiring is another. I would prefer that people submitted GPLv2+, but I don't think it should be a hard requirement. It means, among other things, that we cannot accept most code that originates from the Linux kernel. We could extend this to require unless there is a reason to grant an exception if we wanted to (not saying I know whether we want to or not). I don't want QEMU to be GPLv3. I don't like the terms of the GPLv3. I don't mind GPLv2+, if people want to share code from QEMU in GPLv3 projects, GPLv2+ enables that. The advantage of 100% GPLv2+ (or other GPLv3 compatible) would be that QEMU could share code from GPLv3 projects, specifically latest binutils. Reinventing a disassembler for ever growing x86 assembly is no fun. But if new code is coming in and happens to be under GPLv2, that just means that the contribution cannot be used outside of QEMU in a GPLv3 project. That's fine and that's a decision for the submitter to make. This policy means that we are locked in with GPLv2. Regards, Anthony Liguori -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] Versatile Express: add modelling of NOR flash
Hi, On 09/05/2012 10:47 AM, Peter Maydell wrote: On 5 September 2012 06:16, Stefan Weil s...@weilnetz.de wrote: Am 04.09.2012 19:08, schrieb Francesco Lavra: /* VE_NORFLASH0ALIAS: not modelled */ What about that alias? It's not difficult to add it, too. Just look for memory_region_init_alias in the code to see how it is done (hw/mips_malta.c has an alias region for flash). It's painful because you might also have to add the logic for letting the guest map and unmap the alias (which implies implementing a whole section of the A15 board we don't currently bother with, the SCC registers). I'd need to check the board documentation more carefully to see if we can get away with always mapping that area as the flash alias. Documentation at http://infocenter.arm.com/help/topic/com.arm.doc.ddi0503c/CHDEFDJF.html says that the entire first 512 MB can be mapped to either SMC (which is the default) or AXI, so if AXI is selected neither of the 2 flash banks is visible. Also, the same doc says that it's possible to map either NOR0 (default) or NOR1 to the address 0x. This implies that in the A Series memory map VE_NORFLASH0 should be at 0x0800 and VE_NORFLASH0ALIAS at 0x, not the other way around (by the way, this is also how U-Boot defines the memory for the A5 CoreTile). Maybe worth a patch? If we can get way with always aliasing to flash 0, the actual implementation of the alias is made difficult by the fact that memory_region_init_alias() needs the MemoryRegion of the aliased memory, and the daughterboard-specific initialization is done in a function which doesn't have access to that MemoryRegion. So we can either: 1. move initialization of common flash modelling before daughterboard-specific initialization and pass the relevant MemoryRegion to the daughterboard-specific init function 2. add another field to VEDBoardInfo which tells if the alias capability is implemented, and use this info in vexpress_common_init() to define the alias if appropriate Or we can simply deem this alias not worth the trouble, which is what I thought before sending the patch... Let me know your thoughts. (Also we'd need to fix the current problem with the motherboard address map arrays that there's no way to distinguish peripheral not present on this board from peripheral at address 0, since the A9 board doesn't have the flash alias.) More to the point, this is the third attempt at doing this. Previously Liming Wang sent a patch: http://patchwork.ozlabs.org/patch/147905/ and Jagan sent a two-patch set: http://patchwork.ozlabs.org/patch/171812/ http://patchwork.ozlabs.org/patch/171814/ both of which failed in the code review stage. Francesco, can you check that you haven't fallen into any of the same problems they did, please? I read the reviews of previous attempts, and in fact there is a fix which can be easily done, i.e. replacing the calls to drive_get() with drive_get_next(). Will do that in v2, but first the above points need to be addressed. Thanks, Francesco
[Qemu-devel] [PATCH 14/19] vga: ppm_save(): add error handling
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
hw/blizzard.c | 2 +-
hw/qxl.c| 2 +-
hw/vga.c| 32 +---
hw/vga_int.h| 3 ++-
hw/vmware_vga.c | 2 +-
5 files changed, 30 insertions(+), 11 deletions(-)
diff --git a/hw/blizzard.c b/hw/blizzard.c
index a2b9053..d1c9d81 100644
--- a/hw/blizzard.c
+++ b/hw/blizzard.c
@@ -939,7 +939,7 @@ static void blizzard_screen_dump(void *opaque, const char
*filename,
blizzard_update_display(opaque);
if (s ds_get_data(s-state))
-ppm_save(filename, s-state-surface);
+ppm_save(filename, s-state-surface, errp);
}
#define DEPTH 8
diff --git a/hw/qxl.c b/hw/qxl.c
index 46a929d..bae5758 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1605,7 +1605,7 @@ static void qxl_hw_screen_dump(void *opaque, const char
*filename, bool cswitch,
case QXL_MODE_COMPAT:
case QXL_MODE_NATIVE:
qxl_render_update(qxl);
-ppm_save(filename, qxl-ssd.ds-surface);
+ppm_save(filename, qxl-ssd.ds-surface, errp);
break;
case QXL_MODE_VGA:
vga-screen_dump(vga, filename, cswitch, errp);
diff --git a/hw/vga.c b/hw/vga.c
index dd703cf..80299ea 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -2390,7 +2390,7 @@ void vga_init_vbe(VGACommonState *s, MemoryRegion
*system_memory)
//
/* vga screen dump */
-int ppm_save(const char *filename, struct DisplaySurface *ds)
+void ppm_save(const char *filename, struct DisplaySurface *ds, Error **errp)
{
FILE *f;
uint8_t *d, *d1;
@@ -2402,10 +2402,16 @@ int ppm_save(const char *filename, struct
DisplaySurface *ds)
trace_ppm_save(filename, ds);
f = fopen(filename, wb);
-if (!f)
-return -1;
-fprintf(f, P6\n%d %d\n%d\n,
-ds-width, ds-height, 255);
+if (!f) {
+error_setg(errp, failed to open file '%s': %s, filename,
+ strerror(errno));
+return;
+}
+ret = fprintf(f, P6\n%d %d\n%d\n, ds-width, ds-height, 255);
+if (ret 0) {
+linebuf = NULL;
+goto write_err;
+}
linebuf = g_malloc(ds-width * 3);
d1 = ds-data;
for(y = 0; y ds-height; y++) {
@@ -2426,12 +2432,24 @@ int ppm_save(const char *filename, struct
DisplaySurface *ds)
d += ds-pf.bytes_per_pixel;
}
d1 += ds-linesize;
+clearerr(f);
ret = fwrite(linebuf, 1, pbuf - linebuf, f);
(void)ret;
+if (ferror(f)) {
+goto write_err;
+}
}
+
+out:
g_free(linebuf);
fclose(f);
-return 0;
+return;
+
+write_err:
+error_setg(errp, failed to write to file '%s': %s, filename,
+ strerror(errno));
+unlink(filename);
+goto out;
}
/* save the vga display in a PPM image even if no display is
@@ -2445,5 +2463,5 @@ static void vga_screen_dump(void *opaque, const char
*filename, bool cswitch,
vga_invalidate_display(s);
}
vga_hw_update();
-ppm_save(filename, s-ds-surface);
+ppm_save(filename, s-ds-surface, errp);
}
diff --git a/hw/vga_int.h b/hw/vga_int.h
index 8938093..330a32f 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -23,6 +23,7 @@
*/
#include hw/hw.h
+#include error.h
#include memory.h
#define ST01_V_RETRACE 0x08
@@ -204,7 +205,7 @@ void vga_ioport_write(void *opaque, uint32_t addr, uint32_t
val);
uint32_t vga_mem_readb(VGACommonState *s, target_phys_addr_t addr);
void vga_mem_writeb(VGACommonState *s, target_phys_addr_t addr, uint32_t val);
void vga_invalidate_scanlines(VGACommonState *s, int y1, int y2);
-int ppm_save(const char *filename, struct DisplaySurface *ds);
+void ppm_save(const char *filename, struct DisplaySurface *ds, Error **errp);
int vga_ioport_invalid(VGACommonState *s, uint32_t addr);
void vga_init_vbe(VGACommonState *s, MemoryRegion *address_space);
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 29750e1..b68e883 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -1015,7 +1015,7 @@ static void vmsvga_screen_dump(void *opaque, const char
*filename, bool cswitch,
if (s-depth == 32) {
DisplaySurface *ds = qemu_create_displaysurface_from(s-width,
s-height, 32, ds_get_linesize(s-vga.ds), s-vga.vram_ptr);
-ppm_save(filename, ds);
+ppm_save(filename, ds, errp);
g_free(ds);
}
}
--
1.7.11.2.249.g31c7954.dirty
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
On Tue, Sep 4, 2012 at 9:28 PM, Michael S. Tsirkin m...@redhat.com wrote: On Tue, Sep 04, 2012 at 07:27:32PM +, Blue Swirl wrote: On Tue, Sep 4, 2012 at 8:32 AM, Avi Kivity a...@redhat.com wrote: On 09/03/2012 10:32 PM, Blue Swirl wrote: On Mon, Sep 3, 2012 at 4:14 PM, Avi Kivity a...@redhat.com wrote: On 08/29/2012 11:27 AM, Markus Armbruster wrote: I don't see a point in making contributors avoid non-problems that might conceivably become trivial problems some day. Especially when there's no automated help with the avoiding. -Wpointer-arith +1 FWIW, I'm not in favour of enabling it, just pointing out that it exists. In general I prefer avoiding unnecessary use of extensions, but in this case the extension is trivial and improves readability. Void pointers are not so type safe as uint8_t pointers. casts are even worse. There's also little difference in readability between those in my opinion. here too, casts are worse for readability. I agree they are bad in both accounts, but in most cases it is possible to use different types consistently (like uint8_t * or char * instead of void *) without adding casts. -- error compiling committee.c: too many arguments to function
