Re: [Qemu-devel] [PATCH] cpu-defs.h: pull in qemu-common.h for HOST_LONG_BITS
Am 16.07.2012 07:26, schrieb Stefan Weil: Am 15.07.2012 23:54, schrieb Mike Frysinger: On Sunday 15 July 2012 15:34:33 Stefan Weil wrote: Am 15.07.2012 22:25, schrieb Mike Frysinger: This file uses the define HOST_LONG_BITS, but doesn't explicitly include qemu-common.h for it leading to build warnings for some setups: In file included from qemu/target-bfin/cpu.h:17, from qemu/cputlb.c:21: qemu/cpu-defs.h:83:5: warning: HOST_LONG_BITS is not defined Signed-off-by: Mike Frysinger vap...@gentoo.org --- cpu-defs.h |1 + 1 file changed, 1 insertion(+) diff --git a/cpu-defs.h b/cpu-defs.h index f49e950..0d6018d 100644 --- a/cpu-defs.h +++ b/cpu-defs.h @@ -28,6 +28,7 @@ #include inttypes.h #include signal.h #include osdep.h +#include qemu-common.h #include qemu-queue.h #include targphys.h No. Of course this works, but I don't think that it is reasonable to include qemu-common.h in every *.h file. There are already too many of them. target-bfin/cpu.h should start like all other cpu.h files with these include statements: sorry, but that's fragile junk. if a header file uses defines from another header file, it should be including it. -mike There are different ways how things can be done. Normally, I agree with you that each header file should be complete, but that's not the QEMU style. In your special case, it's more important to keep all */cpu.h similar. qemu/target-bfin/cpu.h is still not part of the official QEMU code, so it can be fixed before it is committed. Cheers, Stefan IMHO it would be also a clean solution if the */cpu.h no longer include config.h and qemu-common.h when those files are included in cpu-def.h. For that solution, your patch could be a starting point, but it needs more cleanup: include statements which are part of qemu-common.h need no duplication in cpu-def.h. - Stefan
Re: [Qemu-devel] [PATCH 10/16] qom: release previous object when setting
Il 15/07/2012 22:25, miny...@acm.org ha scritto: From: Corey Minyard cminy...@mvista.com When setting an object, if you don't release the previous object that was there, it may become unusable. This change allows a chardev to be removed from one object's properties and added to another's. --- qom/object.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/qom/object.c b/qom/object.c index 00bb3b0..484dc77 100644 --- a/qom/object.c +++ b/qom/object.c @@ -731,6 +731,8 @@ void object_property_set(Object *obj, Visitor *v, const char *name, if (!prop-set) { error_set(errp, QERR_PERMISSION_DENIED); } else { + if (prop-release) + prop-release(obj, name, prop-opaque); prop-set(obj, v, prop-opaque, name, errp); } } Acked-by: Paolo Bonzini pbonz...@redhat.com ... but you need to run scripts/checkpatch.pl on your patches to conform to coding style. Thanks! Paolo
[Qemu-devel] Disabling Console Switch using Ctrl + Alt + 2
Hi all, how can I disable a switch to the parallel0 console when accidentially pressing Ctrl + Alt + 2? I don't need such a feature and it confuses some users of the running guest system (some language layouts have the @ placed on the 2 where you need to access either with AltGr or Ctrl + Alt). Best regards, Erik
Re: [Qemu-devel] [PATCH] SCSI: Fail medium writes with proper sense for readonly LUNs
Il 13/07/2012 23:30, Ronnie Sahlberg ha scritto: Add sense code for DATA_PROTECT/WRITE_PROTECTED and return this error for any WRITE*/WRITE_VERIFY* calls if the device is readonly=on, i.e. write-protected Signed-off-by: Ronnie Sahlberg ronniesahlb...@gmail.com Applied, thanks! Paolo --- hw/scsi-bus.c |5 + hw/scsi-disk.c | 16 +--- hw/scsi.h |2 ++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c index 5ad1013..6299094 100644 --- a/hw/scsi-bus.c +++ b/hw/scsi-bus.c @@ -1172,6 +1172,11 @@ const struct SCSISense sense_code_DEVICE_INTERNAL_RESET = { .key = UNIT_ATTENTION, .asc = 0x29, .ascq = 0x04 }; +/* Data Protection, Write Protected */ +const struct SCSISense sense_code_WRITE_PROTECTED = { +.key = DATA_PROTECT, .asc = 0x27, .ascq = 0x00 +}; + /* * scsi_build_sense * diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 2c2be33..0aca383 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -1565,9 +1565,6 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf) r-sector = r-req.cmd.lba * (s-qdev.blocksize / 512); r-sector_count = len * (s-qdev.blocksize / 512); break; -case VERIFY_10: -case VERIFY_12: -case VERIFY_16: case WRITE_6: case WRITE_10: case WRITE_12: @@ -1575,6 +1572,13 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf) case WRITE_VERIFY_10: case WRITE_VERIFY_12: case WRITE_VERIFY_16: +if (bdrv_is_read_only(s-qdev.conf.bs)) { +goto write_protect; +} +/* fallthough */ +case VERIFY_10: +case VERIFY_12: +case VERIFY_16: len = r-req.cmd.xfer / s-qdev.blocksize; DPRINTF(Write %s(sector % PRId64 , count %d)\n, (command 0xe) == 0xe ? And Verify : , @@ -1621,6 +1625,9 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf) DPRINTF(WRITE SAME() (sector % PRId64 , count %d)\n, r-req.cmd.lba, len); +if (bdrv_is_read_only(s-qdev.conf.bs)) { +goto write_protect; +} if (r-req.cmd.lba s-qdev.max_lba) { goto illegal_lba; } @@ -1651,6 +1658,9 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf) illegal_lba: scsi_check_condition(r, SENSE_CODE(LBA_OUT_OF_RANGE)); return 0; +write_protect: +scsi_check_condition(r, SENSE_CODE(WRITE_PROTECTED)); +return 0; } if (r-sector_count == 0 r-iov.iov_len == 0) { scsi_req_complete(r-req, GOOD); diff --git a/hw/scsi.h b/hw/scsi.h index 76f06d4..94d2962 100644 --- a/hw/scsi.h +++ b/hw/scsi.h @@ -202,6 +202,8 @@ extern const struct SCSISense sense_code_MEDIUM_CHANGED; extern const struct SCSISense sense_code_REPORTED_LUNS_CHANGED; /* Unit attention, Device internal reset */ extern const struct SCSISense sense_code_DEVICE_INTERNAL_RESET; +/* Data Protection, Write Protected */ +extern const struct SCSISense sense_code_WRITE_PROTECTED; #define SENSE_CODE(x) sense_code_ ## x
Re: [Qemu-devel] [PATCH 2/6] s390: sclp base support
On 14/07/12 10:48, Blue Swirl wrote: +cpu_physical_memory_write(sccb, work_sccb, + be16_to_cpu(work_sccb.h.length)); Perhaps the DMA helpers should be used instead. Is there any rule what to use under which circumstances. No, it's a recent addition. Devices (at least those which may be used in IOMMU environment) should use DMA helpers for their memory accesses in general but virtio may be an exception. I'm not sure which category this falls in. Ok, I read this as, if we need to handle DMA into virtual addresses at some point in time dma helpers are the way to go. With SCLP we access real storage (no translation) so I think cpu_physical_memory_write is fine here. Christian
Re: [Qemu-devel] [PATCH 12/16] IPMI: Add a PC ISA type structure
Il 15/07/2012 22:25, miny...@acm.org ha scritto: +/* Clear the property from this device so we can put it elsewhere */ +chr = isa-chr; +qdev_prop_set_chr(dev-qdev, chardev, NULL); + +if (chr) { + bdev = qdev_create(NULL, ipmi-bmc-extern); + qdev_prop_set_chr(bdev, chardev, chr); +} else { + bdev = qdev_create(NULL, ipmi-bmc-sim); +} +snprintf(typename, sizeof(typename), ipmi-interface-%s, isa-interface); +idev = qdev_create(isa_bus-qbus, typename); +qdev_prop_set_ptr(bdev, ipmiif, idev); +qdev_prop_set_uint64(idev, iobase, isa-iobase); +qdev_prop_set_ptr(idev, bmc, bdev); +qdev_prop_set_uint8(idev, slave-addr, isa-slave_addr); +rc = qdev_init(idev); I think this is adding a bit of extra complication. You do not need to add explicit devices for this. If you want to make ipmi-bmc-* and ipmi-interface-* objects, you can create them with object_new, set the properties manually with the returned pointer and add the object to the parent with object_property_add_child. Otherwise, it should also be fine if you just add them as function pointers. Either way also lets you keep the ownership of the chardev in the ISA IPMI device, and avoids that info qtree looks strange and does not show the chardev you assigned. Paolo
Re: [Qemu-devel] Disabling Console Switch using Ctrl + Alt + 2
Il 16/07/2012 08:47, Erik Rull ha scritto: how can I disable a switch to the parallel0 console when accidentially pressing Ctrl + Alt + 2? I don't need such a feature and it confuses some users of the running guest system (some language layouts have the @ placed on the 2 where you need to access either with AltGr or Ctrl + Alt). You can just disable the parallel port with -parallel none. Paolo
Re: [Qemu-devel] [RFC] introduce a dynamic library to expose qemu block API
On Fri, Jul 13, 2012 at 10:12:47AM +0100, Stefan Hajnoczi wrote: On Tue, Jul 10, 2012 at 09:18:01AM +0200, Paolo Bonzini wrote: Il 10/07/2012 07:37, Wenchao Xia ha scritto: For getting the other metadata about the disk image you mention, another possibility to is just make 'qemu-img info' return the data in a machine parseable format, ie JSON make a client API for extracting data from this JSON document. Thank u for the idea. The .so is introduced to let program access the image more directly, parsing string is not so fast and it depends on another program's stdout output, I hope to get a faster way. I doubt you actually have profiled it. I think speed is not the issue, instead it's just providing an API that external programs can use. Management tools, backup software, custom administration tools, etc. It's convenient to have an API. Actually I think speed could well be quite relevant. In large deployments it would not be surpising to see 1000's of images in a directory. If you want to be able to query metadata about all of them at once, then being able to open()+read(4k)+close() 1000 times is going to be dramatically faster than doing fork()+execve(qemu-img) 1000 times. I tried to avoid fork() and execv() in the library as much as possible. Dealing with another program's string output will cost quite some codes about its timeout or exception error, more native code seems better to me. NB, I still think qemu-img info should be able to return a JSON parsable data format, regardless of what any block library does. An option to qemu-img to specify output format may be a good way. Daniel -- Best Regards Wenchao Xia
Re: [Qemu-devel] [kvmarm] [PATCH] target-arm: kvm: use KVM_SET_SREGS to set target to Cortex A15
On Fri, 13 Jul 2012 12:06:26 +0200, Alexander Graf ag...@suse.de wrote: struct kvm_sregs { +__u32 target; +__u32 num_features; +__u32 features[14]; }; Are you sure you want to use sregs? We did the mistake of reusing it on ppc, but that doesn't mean you need to repeat the same one :) Basically sregs are an x86 specific struct for its segment register information. I'm quite sure that this is not what your use of them is here. Since each arch is given a hook already, I just abused it. I'll change this to a fresh KVM_ARM_SET_TARGET ioctl. 3) ENABLE_CAP If you only need to enable a feature and care about backwards compatibility of the API (which you don't yet), this is a good one. it basically allows you to enable new features in newer kernel versions which would otherwise break compatibility. You can also pass arbitrary data to ENABLE_CAP to pass in additional information. Hmm, it's not quite a clean fit: this bitmap is for guest features, not kvm ones. Which ones you can enable depends on the target CPU, at least in theory. eg. FP/NEON support and debug register support are (in theory) optional features for an implementation. There may be more in future, I guess. And you really want to initialize this all at the same time; eg. the cpu identification registers need to be initialized depending on the presence of various features. It's also possible that various features may be related, so you can't turn a single one off at a time. Currently, it's a bit theoretical, since we don't have any guest features, but it was suggested that we'll want them in future. Cheers, Rusty.
Re: [Qemu-devel] [PATCH] target-arm: kvm: use KVM_SET_SREGS to set target to Cortex A15
On Fri, 13 Jul 2012 09:06:16 +0100, Peter Maydell peter.mayd...@linaro.org wrote: On 13 July 2012 04:37, Rusty Russell rusty.russ...@linaro.org wrote: Recent kernels use this to set the cpu and features (currently, only the A15 is supported). Note that this causes the registers in the CPU to be initialized, so it's important that all CPUs are created first (they are, as it turns out). This code ignores errors, for backwards compatibility with older kernels. Signed-off-by: Rusty Russell rusty.russ...@linaro.org I haven't actually been posting the ARM KVM patches to qemu-devel thus far, so this patch is a bit of a non-sequitur for qemu-devel readers. (We've been posting patches to kvmarm only, like the kernel patches.) OK; given the wider API implications I thought I'd add them. Anyway: * updates to qemu's kernel headers should be separate patches and ideally the result of running the automatic 'update headers' script. [I'm going to squash all the kernel header changes together before we submit a patch series properly to qemu-devel, so patches which combine header and code changes require me to untangle them] OK. This is totally a hack until we know how we're going to transition. * any code which includes compatibility workarounds for earlier versions of in-development kernel code should have a FIXME comment so we can remember to undo the workaround before submitting. Sure. It's up to you whether you want a flag day: tell me and I'll re-spin the patches? Thanks, Rusty.
Re: [Qemu-devel] [RFC] introduce a dynamic library to expose qemu block API
于 2012-7-13 17:27, Christoph Hellwig 写道: On Fri, Jul 13, 2012 at 10:13:15AM +0100, Stefan Hajnoczi wrote: How is that different from all the qemu-io commands? qemu-io has no modes to just dumb the output without additional information / statistics or for the write case just take user input instead of a pattern. I actually tried to add raw arguments to qemu-io, which still worked ou ok for reads but started to get fairly ugly for the write. What I use in production right now is a trivial qemu-cat tool that just does the raw reads and writes, but I think adding it as a new sub command to qemu-img instead of another tool seems a bit cleaner. If you and Kevin or Anthony disagree and want the qemu-cat tool I can submit a patch for that instead. That seems one user case the library should support. The library is supposed to do more to expose generic block library which qemu device emulator have used, the ideal expect is that other program linked with the library could operate image the same way as qemu core. -- Best Regards Wenchao Xia
Re: [Qemu-devel] [kvmarm] [PATCH] target-arm: kvm: use KVM_SET_SREGS to set target to Cortex A15
On 16.07.2012, at 09:19, Rusty Russell wrote: On Fri, 13 Jul 2012 12:06:26 +0200, Alexander Graf ag...@suse.de wrote: struct kvm_sregs { +__u32 target; +__u32 num_features; +__u32 features[14]; }; Are you sure you want to use sregs? We did the mistake of reusing it on ppc, but that doesn't mean you need to repeat the same one :) Basically sregs are an x86 specific struct for its segment register information. I'm quite sure that this is not what your use of them is here. Since each arch is given a hook already, I just abused it. I'll change this to a fresh KVM_ARM_SET_TARGET ioctl. 3) ENABLE_CAP If you only need to enable a feature and care about backwards compatibility of the API (which you don't yet), this is a good one. it basically allows you to enable new features in newer kernel versions which would otherwise break compatibility. You can also pass arbitrary data to ENABLE_CAP to pass in additional information. Hmm, it's not quite a clean fit: this bitmap is for guest features, not kvm ones. Which ones you can enable depends on the target CPU, at least in theory. Sure. You'd do (pseudo-code): kvm_ioctl(fd, KVM_ENABLE_CAP, KVM_CAP_ARM_CPU_TARGET, ARM_CPU_NEON | ARM_CPU_VFP16 | ARM_CPU_V7); eg. FP/NEON support and debug register support are (in theory) optional features for an implementation. There may be more in future, I guess. And you really want to initialize this all at the same time; eg. the cpu identification registers need to be initialized depending on the presence of various features. It's also possible that various features may be related, so you can't turn a single one off at a time. That argument does hold true. We are in that mess with ppc, where we don't have a proper init ioctl. So you'd definitely be better off defining an extensible init handshake now, so you can easily configure the kernel side early on. Currently for PPC, we just try to either make things stateless or put additional constraints on certain CAPs, like may only be set before the first vcpu run. Not as pretty as doing it cleanly. Currently, it's a bit theoretical, since we don't have any guest features, but it was suggested that we'll want them in future. Yes, hence I'm trying to get you guys to something where you won't be stuck in 6 months from now with a stable ABI that totally doesn't fit you :) Alex
Re: [Qemu-devel] [kvmarm] [PATCH] target-arm: kvm: use KVM_SET_SREGS to set target to Cortex A15
On 07/16/2012 10:19 AM, Rusty Russell wrote: On Fri, 13 Jul 2012 12:06:26 +0200, Alexander Graf ag...@suse.de wrote: struct kvm_sregs { +__u32 target; +__u32 num_features; +__u32 features[14]; }; Are you sure you want to use sregs? We did the mistake of reusing it on ppc, but that doesn't mean you need to repeat the same one :) Basically sregs are an x86 specific struct for its segment register information. I'm quite sure that this is not what your use of them is here. Since each arch is given a hook already, I just abused it. I'll change this to a fresh KVM_ARM_SET_TARGET ioctl. I guess this is equivalent to KVM_SET_CPUID2 on x86. Note that's a vcpu ioctl. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [RFC PATCH 1/4] qapi: Convert savevm
On 07/12/2012 07:53 PM, Eric Blake wrote: On 07/12/2012 10:55 AM, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdinaphrd...@redhat.com --- +++ b/qapi-schema.json @@ -1868,3 +1868,25 @@ # Since: 0.14.0 ## { 'command': 'netdev_del', 'data': {'id': 'str'} } + +## +# @savevm: +# +# Create a snapshot of the whole virtual machine. If 'tag' is provided, +# it is used as human readable identifier. If there is already a snapshot +# with the same tag or ID, it is replaced. +# +# @name: tag or id of new or existing snapshot Needs an #optional designation, given the syntax below. +# +# Returns: Nothing on success +# If there is a writable device not supporting snapshots, +#SnapshotNotSupported +# If no block device can accept snapshots, SnapshotNotAccepted +# If an error occures while creating a snapshot, SnapshotCreateFailed s/occures/occurs/ +# If open a block device for vm state fail, SnapshotOpenFailed +# If an error uccures while writing vm state, SnapshotWriteFailed s/uccures/occurs/ +# If delete snapshot with same 'name' fail, SnapshotDeleteFailed The notion of blindly overwriting the existing snapshot of the same name seems a bit dangerous; should we take this opportunity to enhance the command, and add a force flag, where things fail if the flag is false but the name already exists, and where the reuse only happens if the flag is present? (In fact, it would make my life in libvirt easier, as I have an action item to make libvirt reject attempts to create a snapshot with tag named '1' if an existing snapshot already has an id of '1'.) This sounds reasonable and I think that this could be also good for HMP, not only for QMP. If there isn't anyone who disagree, I'll implement it. Pavel +# +# Since: 1.2 +## +{ 'command': 'savevm', 'data': {'*name': 'str'} } \ No newline at end of file Fix that. @@ -1061,6 +1061,32 @@ Example: EQMP { +.name = savevm, +.args_type = name:s?, +.params = name, +.help = save a VM snapshot. If no tag or id are provided, a new snapshot is created, +.mhandler.cmd_new = qmp_marshal_input_savevm +}, + +SQMP +savevm I know the HMP command is short, for ease of typing; but since 'savevm' is not an English word, should we name the QMP command 'save-vm'?
Re: [Qemu-devel] [RFC PATCH 3/4] qapi: Convert delvm
On 07/12/2012 07:59 PM, Eric Blake wrote: On 07/12/2012 10:55 AM, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdinaphrd...@redhat.com --- hmp-commands.hx |2 +- hmp.c| 10 ++ hmp.h|1 + qapi-schema.json | 17 + qmp-commands.hx | 24 savevm.c | 21 +++-- sysemu.h |1 - 7 files changed, 64 insertions(+), 12 deletions(-) + +## +# @delvm: This name is worse than 'loadvm' or 'savevm', in that we are NOT deleting the entire vm, but a named snapshot stored within the vm. Would naming it 'delete-vm-snapshot' make more sense (in which case the others might make more sense as 'save-vm-snapshot' and 'load-vm-snapshot')? This naming looks nice. I definitely agree that it could be save-vm-snapshot, load-vm-snapshot, delete-vm-snapshot and query-vm-snapshots. Pavel
Re: [Qemu-devel] QEMU VNC Audio - All audio data null
On Mon, Jul 16, 2012 at 03:10:25AM +0100, agraham wrote: On 07/16/2012 01:03 AM, malc wrote: On Sun, 15 Jul 2012, agraham wrote: [..snip..] I've found the root cause and hopefully you should be able to reproduce the issue. There was a configure option introduced called --enable-mixemu. --enable-mixemu enable mixer emulation Try this diff --git a/audio/audio.c b/audio/audio.c index 583ee51..1c77389 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -818,6 +818,7 @@ static int audio_attach_capture (HWVoiceOut *hw) sw-active = hw-enabled; sw-conv = noop_conv; sw-ratio = ((int64_t) hw_cap-info.freq 32) / sw-info.freq; +sw-vol = nominal_volume; sw-rate = st_rate_start (sw-info.freq, hw_cap-info.freq); if (!sw-rate) { dolog (Could not start rate conversion for `%s'\n, SW_NAME (sw)); [..snip..] :) I'm as happy as Larry, works great, Please do file a bug against Fedora for this problem, so that our maintainers sort it out Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH v2 2/3] linux-user: make do_setsockopt support SOL_RAW ICMP_FILTER socket option
On Mon, Jul 16, 2012 at 11:23 AM, Dunrong Huang riegama...@gmail.com wrote: 2012/7/16 Jing Huang jing.huang@gmail.com: This patch makes do_setsockopt() support SOL_RAW ICMP_FILTER socket option. Signed-off-by: Jing Huang jing.huang@gmail.com --- linux-user/syscall.c | 20 1 files changed, 20 insertions(+), 0 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 28c8ba5..fc8690d 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -60,6 +60,7 @@ int __clone2(int (*fn)(void *), void *child_stack_base, #include netinet/ip.h #include netinet/tcp.h #include linux/wireless.h +#include linux/icmp.h #include qemu-common.h #ifdef TARGET_GPROF #include sys/gmon.h @@ -1442,6 +1443,25 @@ static abi_long do_setsockopt(int sockfd, int level, int optname, goto unimplemented; } break; +case SOL_RAW: +switch (optname) { +case ICMP_FILTER: +/*struct icmp_filter takes an u32 value*/ +optname = ICMP_FILTER; Needless assignment statements. optname already has the value ICMP_FILTER do_setsockopt() lacks level=SOL_RAW and optname=ICMP_FILTER. The ping in linux-user guest invokes: setsockopt(icmp_sock, SOL_RAW, ICMP_FILTER, (char *)filt, sizeof(filt)) in main() function. +if (optlen sizeof(uint32_t)) { +return -TARGET_EINVAL; +} + +if (get_user_u32(val, optval_addr)) { +return -TARGET_EFAULT; +} +ret = get_errno(setsockopt(sockfd, level, optname, +(char *)val, sizeof(val))); Dont need casting val to char *. I refer to the definition of setsockopt() in linux-kernel/net/tipc/socket.c +break; +default: +goto unimplemented; +} +break; case TARGET_SOL_SOCKET: switch (optname) { /* Options with 'int' argument. */ -- 1.7.8.6 -- Best Regards, Dunrong Huang
Re: [Qemu-devel] [RFC PATCH 4/4] qapi: Convert info snapshots
On 07/12/2012 08:03 PM, Eric Blake wrote: On 07/12/2012 10:55 AM, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdinaphrd...@redhat.com --- +++ b/qapi-schema.json @@ -934,6 +934,41 @@ { 'command': 'query-block-jobs', 'returns': ['BlockJobInfo'] } ## +# @SnapshotInfo: +# +# Snapshot list. This structure contains list of snapshots for virtual machine. +# +# @id: id of the snapshot. +# +# @tag: human readable tag of the snapshot. Tag was optional on creation, should that field be marked optional and omitted on output when there was no tag on input? If you create snapshot using an ID, Tag is created automatically. Even if it is ugly tag, I think that we shouldn't omit it on output. +# +# @vm_size: size of the snapshot in Bytes. +# +# @date: date and time of the snapshot as timestamp. What type of timestamp? Seconds since Epoch? It is unix timestamp in seconds since Epoch. +# +# @vm_clock: time in the guest in nsecs. If vm_clock is in nsecs, should we also be reporting sub-second resolution in date? Well, I thought that this could be enough, but we could return the same structure as QEMUSnapshotInfo. +SQMP +query-snapshots +-- + +List available snapshots for VM. + +Return a json-object with the following information: Returns an array of json-objects, each with the following information:
Re: [Qemu-devel] [RFC] introduce a dynamic library to expose qemu block API
于 2012-7-13 19:33, Paolo Bonzini 写道: Il 13/07/2012 11:51, Paolo Bonzini ha scritto: Il 13/07/2012 11:16, Stefan Hajnoczi ha scritto: Working around the QEMU block layer license is not a goal per se, especially because you haven't a) assessed _what_ is the GPL code that the library would use; b) told us why the library should not be under the GPL. Please design first according to the functionality you want to implement, then think about the implementation. Licensing is one headache but the real challenge is that the QEMU block layer relies on the QEMU main loop and a bunch of other architecture. It doesn't really, not on Windows which has no AIO for example. That's why I suggested: - assessing what code is GPL and what are the dependencies on it So I tried trimming down the list of files needed to compile qemu tools, and here is a list: Really thanks for the investigation, I paid quite sometime to dig out which license is compatible to LGPL, this have sorted it out. The coroutine and structure inside is quite a challenge. What about provide the library first in nbd + sync access, and waiting for the library employer response? If it is good to use, then replace implement code to native qemu block layer code, change code's license, while keep API unchanged. Easy to relicense to LGPLv2+: block/raw.c none (GPLv2+: Red Hat, IBM) error.c LGPLv2 (Red Hat, IBM, Stefan Weil) iov.c GPLv2 (Red Hat, SuSE/Hannes Reinecke, Michael Tokarev) module.cGPLv2 (Red Hat, IBM, Blue Swirl) qemu-error.cGPLv2+ (Red Hat, Blue Swirl, IBM) trace/control.c GPLv2 (Lluis Vilanova) trace/default.c GPLv2 (Lluis Vilanova) (I added some people to Cc. Lluis and Michael, can you also look at http://wiki.qemu.org/Relicensing if you're willing to relicense your past contributions from GPLv2 to GPLv2+?. Blue Swirl said he'd accept any other GPLv2 or GPLv3 compatible license, which should include LGPLv2+). Harder to relicense to LGPLv2+: block/vdi.c GPLv2+ Good license: aes.c BSD async.c BSD block.c BSD block/bochs.c BSD block/cloop.c BSD block/cow.c BSD block/dmg.c BSD block/parallels.c BSD block/qcow.cBSD block/qcow2-cache.c BSD block/qcow2-cluster.c BSD block/qcow2-refcount.c BSD block/qcow2-snapshot.c BSD block/qcow2.c BSD block/qed-check.c BSD block/qed-cluster.c BSD block/qed-gencb.c BSD block/qed-l2-cache.cBSD block/qed-table.c BSD block/qed.c BSD block/vmdk.cBSD block/vpc.c BSD block/vvfat.c BSD cutils.cBSD osdep.c BSD oslib-posix.c BSD qemu-coroutine-io.c BSD qemu-coroutine-lock.c BSD qemu-option.c BSD qemu-progress.c BSD coroutine-ucontext.cLGPLv2+ json-lexer.cLGPLv2+ json-parser.c LGPLv2+ json-streamer.c LGPLv2+ qbool.c LGPLv2+ qdict.c LGPLv2+ qemu-coroutine.cLGPLv2+ qerror.cLGPLv2+ qfloat.cLGPLv2+ qint.c LGPLv2+ qjson.c LGPLv2+ qlist.c LGPLv2+ qstring.c LGPLv2+ Doesn't need to be included in a library: qemu-tool.c GPLv2 Autogenerated: trace.c Remaining undefined symbols: qemu_aio_flush qemu_aio_wait qemu_free_timer qemu_new_timer qemu_mod_timer qemu_del_timer qemu_get_clock_ns vm_clock + those defined in qemu-tool.c Paolo -- Best Regards Wenchao Xia
Re: [Qemu-devel] [RFC] introduce a dynamic library to expose qemu block API
Il 16/07/2012 10:16, Wenchao Xia ha scritto: Really thanks for the investigation, I paid quite sometime to dig out which license is compatible to LGPL, this have sorted it out. The coroutine and structure inside is quite a challenge. Coroutines are really just a small complication in the program flow if all you support is synchronous access to files (i.e. no HTTP etc.). Their usage should be completely transparent. What about provide the library first in nbd + sync access, and waiting for the library employer response? If it is good to use, then replace implement code to native qemu block layer code, change code's license, while keep API unchanged. You can start by proposing the API. Paolo
[Qemu-devel] [PATCH] fdc: fix relative seek
Signed-off-by: Pavel Hrdina phrd...@redhat.com --- hw/fdc.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/fdc.c b/hw/fdc.c index edf0706..decb1f7 100644 --- a/hw/fdc.c +++ b/hw/fdc.c @@ -1705,7 +1705,8 @@ static void fdctrl_handle_relative_seek_out(FDCtrl *fdctrl, int direction) fd_seek(cur_drv, cur_drv-head, cur_drv-max_track - 1, cur_drv-sect, 1); } else { -fd_seek(cur_drv, cur_drv-head, fdctrl-fifo[2], cur_drv-sect, 1); +fd_seek(cur_drv, cur_drv-head, +cur_drv-track + fdctrl-fifo[2], cur_drv-sect, 1); } fdctrl_reset_fifo(fdctrl); /* Raise Interrupt */ @@ -1721,7 +1722,8 @@ static void fdctrl_handle_relative_seek_in(FDCtrl *fdctrl, int direction) if (fdctrl-fifo[2] cur_drv-track) { fd_seek(cur_drv, cur_drv-head, 0, cur_drv-sect, 1); } else { -fd_seek(cur_drv, cur_drv-head, fdctrl-fifo[2], cur_drv-sect, 1); +fd_seek(cur_drv, cur_drv-head, +cur_drv-track - fdctrl-fifo[2], cur_drv-sect, 1); } fdctrl_reset_fifo(fdctrl); /* Raise Interrupt */ -- 1.7.10.4
Re: [Qemu-devel] [RFC] introduce a dynamic library to expose qemu block API
On Sat, Jul 14, 2012 at 01:55:07AM +0300, Lluís Vilanova wrote: Paolo Bonzini writes: Il 13/07/2012 11:51, Paolo Bonzini ha scritto: Il 13/07/2012 11:16, Stefan Hajnoczi ha scritto: Working around the QEMU block layer license is not a goal per se, especially because you haven't a) assessed _what_ is the GPL code that the library would use; b) told us why the library should not be under the GPL. Please design first according to the functionality you want to implement, then think about the implementation. Licensing is one headache but the real challenge is that the QEMU block layer relies on the QEMU main loop and a bunch of other architecture. It doesn't really, not on Windows which has no AIO for example. That's why I suggested: - assessing what code is GPL and what are the dependencies on it So I tried trimming down the list of files needed to compile qemu tools, and here is a list: Easy to relicense to LGPLv2+: block/raw.c none (GPLv2+: Red Hat, IBM) error.c LGPLv2 (Red Hat, IBM, Stefan Weil) iov.c GPLv2 (Red Hat, SuSE/Hannes Reinecke, Michael Tokarev) module.cGPLv2 (Red Hat, IBM, Blue Swirl) qemu-error.cGPLv2+ (Red Hat, Blue Swirl, IBM) trace/control.c GPLv2 (Lluis Vilanova) trace/default.c GPLv2 (Lluis Vilanova) (I added some people to Cc. Lluis and Michael, can you also look at http://wiki.qemu.org/Relicensing if you're willing to relicense your past contributions from GPLv2 to GPLv2+?. Blue Swirl said he'd accept any other GPLv2 or GPLv3 compatible license, which should include LGPLv2+). I have no problems relicensing to GPLv2 or later or GPLv3 or later. What about LGPLv2+? (Note the L.) Stefan
[Qemu-devel] [PATCH 00/12] Portable thread-pool/AIO, Win32 emulated AIO
This patch series is part 2 in my EventNotifier/AIO improvements for QEMU 1.2. It extends use of EventNotifier to the main loop and AIO subsystems. A new API using EventNotifier is added to aio.c and a new portable thread pool is introduced (based on code from posix-aio-compat.c, mostly) that uses this API. raw-posix.c is converted to use the new thread pool, and support for asynchronous I/O is finally added to Win32 as well. The network drivers (curl, libiscsi, nbd) have to be disabled under Windows. I tested this under Wine, with a RHEL virtual machine booting just as glacially as before. However, info blockstats does show a slightly higher overhead, so I would like this to be tested on real Windows hosts. However, even if the result is negative, I would prefer to keep the early parts (i.e. drop only the last patch) since they are a prerequisite for more improvements to block/raw-posix.c (such as asynchronous discard support). Paolo Bonzini (12): event_notifier: enable it to use pipes event_notifier: add Win32 implementation main-loop: use event notifiers aio: provide platform-independent API aio: add Win32 implementation linux-aio: use event notifiers qemu-thread: add QemuSemaphore aio: add generic thread-pool facility block: switch posix-aio-compat to threadpool raw: merge posix-aio-compat.c into block/raw-posix.c raw-posix: rename raw-posix-aio.h, hide unavailable prototypes raw-win32: add emulated AIO support Makefile.objs| 13 +- aio.c = aio-posix.c |9 + aio-win32.c | 177 + block/Makefile.objs |6 +- block/{raw-posix-aio.h = raw-aio.h} | 19 +- block/raw-posix.c| 301 ++- block/raw-win32.c| 189 +++--- event_notifier-posix.c | 118 ++ event_notifier-win32.c | 59 +++ event_notifier.c | 67 event_notifier.h | 20 +- linux-aio.c | 51 +-- main-loop.c | 106 +- oslib-posix.c| 31 -- posix-aio-compat.c | 681 -- qemu-aio.h | 19 +- qemu-common.h|1 - qemu-thread-posix.c | 74 qemu-thread-posix.h |5 + qemu-thread-win32.c | 35 ++ qemu-thread-win32.h |4 + qemu-thread.h|7 + thread-pool.c| 279 ++ thread-pool.h| 34 ++ trace-events |5 + 25 files changed, 1329 insertions(+), 981 deletions(-) rename aio.c = aio-posix.c (92%) create mode 100644 aio-win32.c rename block/{raw-posix-aio.h = raw-aio.h} (62%) create mode 100644 event_notifier-posix.c create mode 100644 event_notifier-win32.c delete mode 100644 event_notifier.c delete mode 100644 posix-aio-compat.c create mode 100644 thread-pool.c create mode 100644 thread-pool.h -- 1.7.10.4
[Qemu-devel] [PATCH 01/12] event_notifier: enable it to use pipes
This takes the eventfd emulation code from the main loop and adds it to EventNotifier. When the EventNotifier is used for the main loop too, we need this compatibility code. Without CONFIG_EVENTFD, event_notifier_get_fd is only usable for the read side of the notifier, for example to set a select() handler. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- event_notifier.c | 83 +++--- event_notifier.h |3 +- 2 files changed, 69 insertions(+), 17 deletions(-) diff --git a/event_notifier.c b/event_notifier.c index 2c207e1..dde2d32 100644 --- a/event_notifier.c +++ b/event_notifier.c @@ -20,48 +20,99 @@ void event_notifier_init_fd(EventNotifier *e, int fd) { -e-fd = fd; +e-rfd = fd; +e-wfd = fd; } int event_notifier_init(EventNotifier *e, int active) { +int fds[2]; +int ret; + #ifdef CONFIG_EVENTFD -int fd = eventfd(!!active, EFD_NONBLOCK | EFD_CLOEXEC); -if (fd 0) -return -errno; -e-fd = fd; -return 0; +ret = eventfd(0, O_NONBLOCK); #else -return -ENOSYS; +ret = -1; +errno = ENOSYS; #endif +if (ret = 0) { +e-rfd = e-wfd = ret; +qemu_set_cloexec(ret); +} else { +if (errno != ENOSYS) { +return -errno; +} +if (qemu_pipe(fds) 0) { +return -errno; +} +ret = fcntl_setfl(fds[0], O_NONBLOCK); +if (ret 0) { +goto fail; +} +ret = fcntl_setfl(fds[1], O_NONBLOCK); +if (ret 0) { +goto fail; +} +e-rfd = fds[0]; +e-wfd = fds[1]; +} +if (active) +event_notifier_set(e); +return 0; + +fail: +close(fds[0]); +close(fds[1]); +return ret; } void event_notifier_cleanup(EventNotifier *e) { -close(e-fd); +if (e-rfd != e-wfd) { +close(e-rfd); +} +close(e-wfd); } int event_notifier_get_fd(EventNotifier *e) { -return e-fd; +return e-rfd; } int event_notifier_set_handler(EventNotifier *e, EventNotifierHandler *handler) { -return qemu_set_fd_handler(e-fd, (IOHandler *)handler, NULL, e); +return qemu_set_fd_handler(e-rfd, (IOHandler *)handler, NULL, e); } int event_notifier_set(EventNotifier *e) { -uint64_t value = 1; -int r = write(e-fd, value, sizeof(value)); -return r == sizeof(value); +static const uint64_t value = 1; +ssize_t ret; + +do { +ret = write(e-wfd, value, sizeof(value)); +} while (ret 0 errno == EINTR); + +/* EAGAIN is fine, a read must be pending. */ +if (ret 0 errno != EAGAIN) { +return -1; +} +return 0; } int event_notifier_test_and_clear(EventNotifier *e) { -uint64_t value; -int r = read(e-fd, value, sizeof(value)); -return r == sizeof(value); +int value; +ssize_t len; +char buffer[512]; + +/* Drain the notify pipe. For eventfd, only 8 bytes will be read. */ +value = 0; +do { +len = read(e-rfd, buffer, sizeof(buffer)); +value |= (len 0); +} while ((len == -1 errno == EINTR) || len == sizeof(buffer)); + +return value; } diff --git a/event_notifier.h b/event_notifier.h index f0ec2f2..f04d12d 100644 --- a/event_notifier.h +++ b/event_notifier.h @@ -16,7 +16,8 @@ #include qemu-common.h struct EventNotifier { -int fd; +int rfd; +int wfd; }; typedef void EventNotifierHandler(EventNotifier *); -- 1.7.10.4
[Qemu-devel] [PATCH 12/12] raw-win32: add emulated AIO support
The thread pool can be used under Win32 in the same way as in raw-posix.c. Move the existing synchronous code into callbacks, and pass the return code back. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- block/raw-win32.c | 189 +++-- 1 file changed, 140 insertions(+), 49 deletions(-) diff --git a/block/raw-win32.c b/block/raw-win32.c index e4b0b75..a50d636 100644 --- a/block/raw-win32.c +++ b/block/raw-win32.c @@ -25,6 +25,9 @@ #include qemu-timer.h #include block_int.h #include module.h +#include raw-aio.h +#include trace.h +#include thread-pool.h #include windows.h #include winioctl.h @@ -32,12 +35,130 @@ #define FTYPE_CD 1 #define FTYPE_HARDDISK 2 +struct qemu_paiocb { +BlockDriverState *bs; +HANDLE hfile; +struct iovec *aio_iov; +int aio_niov; +size_t aio_nbytes; +off_t aio_offset; +int aio_type; +}; + typedef struct BDRVRawState { HANDLE hfile; int type; char drive_path[16]; /* format: d:\ */ } BDRVRawState; +/* + * Read/writes the data to/from a given linear buffer. + * + * Returns the number of bytes handles or -errno in case of an error. Short + * reads are only returned if the end of the file is reached. + */ +static size_t handle_aiocb_rw(struct qemu_paiocb *aiocb) +{ +size_t offset = 0; +int i; + +for (i = 0; i aiocb-aio_niov; i++) { +OVERLAPPED ov; +DWORD ret, ret_count, len; + +memset(ov, 0, sizeof(ov)); +ov.Offset = (aiocb-aio_offset + offset); +ov.OffsetHigh = (aiocb-aio_offset + offset) 32; +len = aiocb-aio_iov[i].iov_len; +if (aiocb-aio_type QEMU_AIO_WRITE) { +ret = WriteFile(aiocb-hfile, aiocb-aio_iov[i].iov_base, +len, ret_count, ov); +} else { +ret = ReadFile(aiocb-hfile, aiocb-aio_iov[i].iov_base, + len, ret_count, ov); +} +if (!ret) { +ret_count = 0; +} +if (ret_count != len) { +break; +} +offset += len; +} + +return offset; +} + +static int aio_worker(void *arg) +{ +struct qemu_paiocb *aiocb = arg; +ssize_t ret = 0; +size_t count; + +switch (aiocb-aio_type QEMU_AIO_TYPE_MASK) { +case QEMU_AIO_READ: +count = handle_aiocb_rw(aiocb); +if (count aiocb-aio_nbytes aiocb-bs-growable) { +/* A short read means that we have reached EOF. Pad the buffer + * with zeros for bytes after EOF. */ +QEMUIOVector qiov; + +qemu_iovec_init_external(qiov, aiocb-aio_iov, + aiocb-aio_niov); +qemu_iovec_memset_skip(qiov, 0, aiocb-aio_nbytes - count, count); + +count = aiocb-aio_nbytes; +} +if (count == aiocb-aio_nbytes) { +ret = 0; +} else { +ret = -EINVAL; +} +break; +case QEMU_AIO_WRITE: +count = handle_aiocb_rw(aiocb); +if (count == aiocb-aio_nbytes) { +count = 0; +} else { +count = -EINVAL; +} +break; +case QEMU_AIO_FLUSH: +if (!FlushFileBuffers(aiocb-hfile)) { +return -EIO; +} +break; +default: +fprintf(stderr, invalid aio request (0x%x)\n, aiocb-aio_type); +ret = -EINVAL; +break; +} + +g_slice_free(struct qemu_paiocb, aiocb); +return ret; +} + +static BlockDriverAIOCB *paio_submit(BlockDriverState *bs, HANDLE hfile, +int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, +BlockDriverCompletionFunc *cb, void *opaque, int type) +{ +struct qemu_paiocb *acb = g_slice_new(struct qemu_paiocb); + +acb-bs = bs; +acb-hfile = hfile; +acb-aio_type = type; + +if (qiov) { +acb-aio_iov = qiov-iov; +acb-aio_niov = qiov-niov; +} +acb-aio_nbytes = nb_sectors * 512; +acb-aio_offset = sector_num * 512; + +trace_paio_submit(acb, opaque, sector_num, nb_sectors, type); +return thread_pool_submit_aio(aio_worker, acb, cb, opaque); +} + int qemu_ftruncate64(int fd, int64_t length) { LARGE_INTEGER li; @@ -109,59 +230,29 @@ static int raw_open(BlockDriverState *bs, const char *filename, int flags) return 0; } -static int raw_read(BlockDriverState *bs, int64_t sector_num, -uint8_t *buf, int nb_sectors) +static BlockDriverAIOCB *raw_aio_readv(BlockDriverState *bs, + int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, + BlockDriverCompletionFunc *cb, void *opaque) { BDRVRawState *s = bs-opaque; -OVERLAPPED ov; -DWORD ret_count; -int ret; -int64_t offset = sector_num * 512; -int count = nb_sectors * 512; - -memset(ov, 0, sizeof(ov)); -ov.Offset = offset; -ov.OffsetHigh = offset 32; -ret = ReadFile(s-hfile, buf, count, ret_count,
[Qemu-devel] [PATCH 09/12] block: switch posix-aio-compat to threadpool
This is not meant for portability, but to remove code duplication. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- block/raw-posix-aio.h |1 - block/raw-posix.c |5 - posix-aio-compat.c| 433 + 3 files changed, 42 insertions(+), 397 deletions(-) diff --git a/block/raw-posix-aio.h b/block/raw-posix-aio.h index ba118f6..6725135 100644 --- a/block/raw-posix-aio.h +++ b/block/raw-posix-aio.h @@ -28,7 +28,6 @@ /* posix-aio-compat.c - thread pool based implementation */ -int paio_init(void); BlockDriverAIOCB *paio_submit(BlockDriverState *bs, int fd, int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, BlockDriverCompletionFunc *cb, void *opaque, int type); diff --git a/block/raw-posix.c b/block/raw-posix.c index 0dce089..e5faccd 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -234,11 +234,6 @@ static int raw_open_common(BlockDriverState *bs, const char *filename, } } -/* We're falling back to POSIX AIO in some cases so init always */ -if (paio_init() 0) { -goto out_free_buf; -} - #ifdef CONFIG_LINUX_AIO /* * Currently Linux do AIO only for files opened with O_DIRECT diff --git a/posix-aio-compat.c b/posix-aio-compat.c index 68361f5..cf716dc 100644 --- a/posix-aio-compat.c +++ b/posix-aio-compat.c @@ -28,12 +28,11 @@ #include sysemu.h #include qemu-common.h #include trace.h +#include thread-pool.h #include block_int.h #include block/raw-posix-aio.h -static void do_spawn_thread(void); - struct qemu_paiocb { BlockDriverAIOCB common; int aio_fildes; @@ -45,82 +44,15 @@ struct qemu_paiocb { size_t aio_nbytes; #define aio_ioctl_cmd aio_nbytes /* for QEMU_AIO_IOCTL */ off_t aio_offset; - -QTAILQ_ENTRY(qemu_paiocb) node; int aio_type; -ssize_t ret; -int active; -struct qemu_paiocb *next; }; -typedef struct PosixAioState { -int rfd, wfd; -struct qemu_paiocb *first_aio; -} PosixAioState; - - -static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; -static pthread_cond_t cond = PTHREAD_COND_INITIALIZER; -static pthread_t thread_id; -static pthread_attr_t attr; -static int max_threads = 64; -static int cur_threads = 0; -static int idle_threads = 0; -static int new_threads = 0; /* backlog of threads we need to create */ -static int pending_threads = 0; /* threads created but not running yet */ -static QEMUBH *new_thread_bh; -static QTAILQ_HEAD(, qemu_paiocb) request_list; - #ifdef CONFIG_PREADV static int preadv_present = 1; #else static int preadv_present = 0; #endif -static void die2(int err, const char *what) -{ -fprintf(stderr, %s failed: %s\n, what, strerror(err)); -abort(); -} - -static void die(const char *what) -{ -die2(errno, what); -} - -static void mutex_lock(pthread_mutex_t *mutex) -{ -int ret = pthread_mutex_lock(mutex); -if (ret) die2(ret, pthread_mutex_lock); -} - -static void mutex_unlock(pthread_mutex_t *mutex) -{ -int ret = pthread_mutex_unlock(mutex); -if (ret) die2(ret, pthread_mutex_unlock); -} - -static int cond_timedwait(pthread_cond_t *cond, pthread_mutex_t *mutex, - struct timespec *ts) -{ -int ret = pthread_cond_timedwait(cond, mutex, ts); -if (ret ret != ETIMEDOUT) die2(ret, pthread_cond_timedwait); -return ret; -} - -static void cond_signal(pthread_cond_t *cond) -{ -int ret = pthread_cond_signal(cond); -if (ret) die2(ret, pthread_cond_signal); -} - -static void thread_create(pthread_t *thread, pthread_attr_t *attr, - void *(*start_routine)(void*), void *arg) -{ -int ret = pthread_create(thread, attr, start_routine, arg); -if (ret) die2(ret, pthread_create); -} - static ssize_t handle_aiocb_ioctl(struct qemu_paiocb *aiocb) { int ret; @@ -309,289 +241,57 @@ static ssize_t handle_aiocb_rw(struct qemu_paiocb *aiocb) return nbytes; } -static void posix_aio_notify_event(void); - -static void *aio_thread(void *unused) +static int aio_worker(void *arg) { -mutex_lock(lock); -pending_threads--; -mutex_unlock(lock); -do_spawn_thread(); - -while (1) { -struct qemu_paiocb *aiocb; -ssize_t ret = 0; -qemu_timeval tv; -struct timespec ts; - -qemu_gettimeofday(tv); -ts.tv_sec = tv.tv_sec + 10; -ts.tv_nsec = 0; - -mutex_lock(lock); - -while (QTAILQ_EMPTY(request_list) - !(ret == ETIMEDOUT)) { -idle_threads++; -ret = cond_timedwait(cond, lock, ts); -idle_threads--; +struct qemu_paiocb *aiocb = arg; +ssize_t ret = 0; + +switch (aiocb-aio_type QEMU_AIO_TYPE_MASK) { +case QEMU_AIO_READ: +ret = handle_aiocb_rw(aiocb); +if (ret = 0 ret aiocb-aio_nbytes aiocb-common.bs-growable) { +/* A short read means that we have reached EOF. Pad the buffer + * with zeros for
Re: [Qemu-devel] QEMU VNC Audio - All audio data null
On 07/16/2012 09:12 AM, Daniel P. Berrange wrote: On Mon, Jul 16, 2012 at 03:10:25AM +0100, agraham wrote: On 07/16/2012 01:03 AM, malc wrote: On Sun, 15 Jul 2012, agraham wrote: [..snip..] I've found the root cause and hopefully you should be able to reproduce the issue. There was a configure option introduced called --enable-mixemu. --enable-mixemu enable mixer emulation Try this diff --git a/audio/audio.c b/audio/audio.c index 583ee51..1c77389 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -818,6 +818,7 @@ static int audio_attach_capture (HWVoiceOut *hw) sw-active = hw-enabled; sw-conv = noop_conv; sw-ratio = ((int64_t) hw_cap-info.freq 32) / sw-info.freq; +sw-vol = nominal_volume; sw-rate = st_rate_start (sw-info.freq, hw_cap-info.freq); if (!sw-rate) { dolog (Could not start rate conversion for `%s'\n, SW_NAME (sw)); [..snip..] :) I'm as happy as Larry, works great, Please do file a bug against Fedora for this problem, so that our maintainers sort it out Daniel Will do. Hey, your blog rocks! Albert
[Qemu-devel] [PATCH 08/12] aio: add generic thread-pool facility
Add a generic thread-pool. The code is roughly based on posix-aio-compat.c, with some changes, especially the following: - use QemuSemaphore instead of QemuCond; - separate the state of the thread from the return code of the worker function. The return code is totally opaque for the thread pool; - do not busy wait when doing cancellation. A more generic threadpool (but still specific to I/O so that in the future it can use special scheduling classes or PI mutexes) can have many uses: it allows more flexibility in raw-posix.c and can more easily be extended to Win32, and it will also be used to do an msync of the persistent bitmap. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- Makefile.objs |2 +- thread-pool.c | 279 + thread-pool.h | 34 +++ trace-events |5 ++ 4 files changed, 319 insertions(+), 1 deletion(-) create mode 100644 thread-pool.c create mode 100644 thread-pool.h diff --git a/Makefile.objs b/Makefile.objs index 96d0e68..9a30cb5 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -41,7 +41,7 @@ coroutine-obj-$(CONFIG_WIN32) += coroutine-win32.o ### # block-obj-y is code used by both qemu system emulation and qemu-img -block-obj-y = cutils.o cache-utils.o qemu-option.o module.o async.o +block-obj-y = cutils.o cache-utils.o qemu-option.o module.o async.o thread-pool.o block-obj-y += nbd.o block.o aes.o qemu-config.o qemu-progress.o qemu-sockets.o block-obj-y += $(coroutine-obj-y) $(qobject-obj-y) $(version-obj-y) block-obj-$(CONFIG_POSIX) += posix-aio-compat.o diff --git a/thread-pool.c b/thread-pool.c new file mode 100644 index 000..7895544 --- /dev/null +++ b/thread-pool.c @@ -0,0 +1,279 @@ +/* + * QEMU block layer thread pool + * + * Copyright IBM, Corp. 2008 + * Copyright Red Hat, Inc. 2012 + * + * Authors: + * Anthony Liguori aligu...@us.ibm.com + * Paolo Bonzini pbonz...@redhat.com + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + * + * Contributions after 2012-01-13 are licensed under the terms of the + * GNU GPL, version 2 or (at your option) any later version. + */ +#include qemu-common.h +#include qemu-queue.h +#include qemu-thread.h +#include osdep.h +#include qemu-coroutine.h +#include trace.h +#include block_int.h +#include event_notifier.h +#include thread-pool.h + +static void do_spawn_thread(void); + +typedef struct ThreadPoolElement ThreadPoolElement; + +enum ThreadState { +THREAD_QUEUED, +THREAD_ACTIVE, +THREAD_DONE, +THREAD_CANCELED, +}; + +struct ThreadPoolElement { +BlockDriverAIOCB common; +ThreadPoolFunc *func; +void *arg; +enum ThreadState state; +int ret; + +QTAILQ_ENTRY(ThreadPoolElement) reqs; +QLIST_ENTRY(ThreadPoolElement) all; +}; + +static EventNotifier notifier; +static QemuMutex lock; +static QemuCond check_cancel; +static QemuSemaphore sem; +static int max_threads = 64; +static int cur_threads = 0; +static int idle_threads = 0; +static int new_threads = 0; /* backlog of threads we need to create */ +static int pending_threads = 0; /* threads created but not running yet */ +static int pending_cancellations = 0; /* whether we need a cond_broadcast */ +static QEMUBH *new_thread_bh; +static QLIST_HEAD(, ThreadPoolElement) head; +static QTAILQ_HEAD(, ThreadPoolElement) request_list; + +static void *worker_thread(void *unused) +{ +qemu_mutex_lock(lock); +pending_threads--; +qemu_mutex_unlock(lock); +do_spawn_thread(); + +while (1) { +ThreadPoolElement *req; +int ret; + +qemu_mutex_lock(lock); +idle_threads++; +qemu_mutex_unlock(lock); +ret = qemu_sem_timedwait(sem, 1); +qemu_mutex_lock(lock); +idle_threads--; +if (ret == -1) { +if (QTAILQ_EMPTY(request_list)) { +break; +} +qemu_mutex_unlock(lock); +continue; +} + +req = QTAILQ_FIRST(request_list); +QTAILQ_REMOVE(request_list, req, reqs); +req-state = THREAD_ACTIVE; +qemu_mutex_unlock(lock); + +ret = req-func(req-arg); + +qemu_mutex_lock(lock); +req-state = THREAD_DONE; +req-ret = ret; +if (pending_cancellations) { +qemu_cond_broadcast(check_cancel); +} +qemu_mutex_unlock(lock); + +event_notifier_set(notifier); +} + +cur_threads--; +qemu_mutex_unlock(lock); + +return NULL; +} + +static void do_spawn_thread(void) +{ +QemuThread t; + +qemu_mutex_lock(lock); +if (!new_threads) { +qemu_mutex_unlock(lock); +return; +} + +new_threads--; +pending_threads++; + +qemu_mutex_unlock(lock); + +qemu_thread_create(t, worker_thread, NULL, QEMU_THREAD_DETACHED); +} + +static void spawn_thread_bh_fn(void
Re: [Qemu-devel] [PATCH] configure: Fix build with ALSA audio driver
On Sun, Jul 15, 2012 at 03:49:13PM +0200, Stefan Weil wrote: One of the buildbot jobs should be configured with all sound options: --audio-card-list=ac97,es1370,sb16,cs4231a,adlib,gus,hda --audio-drv-list=alsa,sdl,oss,esd,pa --enable-mixemu That would have detected this bug (and more potential bugs in the future). Daniel, please could you add these ./configure options to the qemu.git buildbot? I have tested on the yuzuki buildbot with Stefan Weil's patch applied. Yuzuki has the library headers installed so we can build it all and get better coverage. Thanks, Stefan
Re: [Qemu-devel] [PATCH] configure: adapt vde test to -Werror
On Sun, Jul 15, 2012 at 09:38:25PM +0200, Laszlo Ersek wrote: ... after commit 417c9d72 (configure: add -Werror to QEMU_CFLAGS early) Signed-off-by: Laszlo Ersek ler...@redhat.com --- configure |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) Thanks, applied to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
[Qemu-devel] [PATCH 10/12] raw: merge posix-aio-compat.c into block/raw-posix.c
Making the qemu_paiocb specific to raw devices will let us access members of the BDRVRawState arbitrarily. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- Makefile.objs |1 - block/raw-posix-aio.h |8 -- block/raw-posix.c | 294 +++ posix-aio-compat.c| 332 - 4 files changed, 294 insertions(+), 341 deletions(-) delete mode 100644 posix-aio-compat.c diff --git a/Makefile.objs b/Makefile.objs index 9a30cb5..3debcba 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -44,7 +44,6 @@ coroutine-obj-$(CONFIG_WIN32) += coroutine-win32.o block-obj-y = cutils.o cache-utils.o qemu-option.o module.o async.o thread-pool.o block-obj-y += nbd.o block.o aes.o qemu-config.o qemu-progress.o qemu-sockets.o block-obj-y += $(coroutine-obj-y) $(qobject-obj-y) $(version-obj-y) -block-obj-$(CONFIG_POSIX) += posix-aio-compat.o block-obj-$(CONFIG_POSIX) += event_notifier-posix.o aio-posix.o block-obj-$(CONFIG_WIN32) += event_notifier-win32.o aio-win32.o block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o diff --git a/block/raw-posix-aio.h b/block/raw-posix-aio.h index 6725135..c714367 100644 --- a/block/raw-posix-aio.h +++ b/block/raw-posix-aio.h @@ -27,14 +27,6 @@ #define QEMU_AIO_MISALIGNED 0x1000 -/* posix-aio-compat.c - thread pool based implementation */ -BlockDriverAIOCB *paio_submit(BlockDriverState *bs, int fd, -int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, -BlockDriverCompletionFunc *cb, void *opaque, int type); -BlockDriverAIOCB *paio_ioctl(BlockDriverState *bs, int fd, -unsigned long int req, void *buf, -BlockDriverCompletionFunc *cb, void *opaque); - /* linux-aio.c - Linux native implementation */ void *laio_init(void); BlockDriverAIOCB *laio_submit(BlockDriverState *bs, void *aio_ctx, int fd, diff --git a/block/raw-posix.c b/block/raw-posix.c index e5faccd..3e2e822 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -27,6 +27,8 @@ #include qemu-log.h #include block_int.h #include module.h +#include trace.h +#include thread-pool.h #include block/raw-posix-aio.h #if defined(__APPLE__) (__MACH__) @@ -143,6 +145,20 @@ typedef struct BDRVRawState { static int fd_open(BlockDriverState *bs); static int64_t raw_getlength(BlockDriverState *bs); +struct qemu_paiocb { +BlockDriverState *bs; +int aio_fildes; +union { +struct iovec *aio_iov; +void *aio_ioctl_buf; +}; +int aio_niov; +size_t aio_nbytes; +#define aio_ioctl_cmd aio_nbytes /* for QEMU_AIO_IOCTL */ +off_t aio_offset; +int aio_type; +}; + #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) static int cdrom_reopen(BlockDriverState *bs); #endif @@ -311,6 +327,284 @@ static int qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov) return 1; } +static ssize_t handle_aiocb_ioctl(struct qemu_paiocb *aiocb) +{ +int ret; + +ret = ioctl(aiocb-aio_fildes, aiocb-aio_ioctl_cmd, aiocb-aio_ioctl_buf); +if (ret == -1) +return -errno; + +/* + * This looks weird, but the aio code only considers a request + * successful if it has written the full number of bytes. + * + * Now we overload aio_nbytes as aio_ioctl_cmd for the ioctl command, + * so in fact we return the ioctl command here to make posix_aio_read() + * happy.. + */ +return aiocb-aio_nbytes; +} + +static ssize_t handle_aiocb_flush(struct qemu_paiocb *aiocb) +{ +int ret; + +ret = qemu_fdatasync(aiocb-aio_fildes); +if (ret == -1) +return -errno; +return 0; +} + +#ifdef CONFIG_PREADV + +static int preadv_present = 1; + +static ssize_t +qemu_preadv(int fd, const struct iovec *iov, int nr_iov, off_t offset) +{ +return preadv(fd, iov, nr_iov, offset); +} + +static ssize_t +qemu_pwritev(int fd, const struct iovec *iov, int nr_iov, off_t offset) +{ +return pwritev(fd, iov, nr_iov, offset); +} + +#else + +static int preadv_present = 0; + +static ssize_t +qemu_preadv(int fd, const struct iovec *iov, int nr_iov, off_t offset) +{ +return -ENOSYS; +} + +static ssize_t +qemu_pwritev(int fd, const struct iovec *iov, int nr_iov, off_t offset) +{ +return -ENOSYS; +} + +#endif + +static ssize_t handle_aiocb_rw_vector(struct qemu_paiocb *aiocb) +{ +ssize_t len; + +do { +if (aiocb-aio_type QEMU_AIO_WRITE) +len = qemu_pwritev(aiocb-aio_fildes, + aiocb-aio_iov, + aiocb-aio_niov, + aiocb-aio_offset); + else +len = qemu_preadv(aiocb-aio_fildes, + aiocb-aio_iov, + aiocb-aio_niov, + aiocb-aio_offset); +} while (len == -1 errno == EINTR); + +if (len == -1) +return -errno; +return len; +} + +/* + * Read/writes the data to/from a given linear buffer. + * + * Returns
Re: [Qemu-devel] [PATCH 2/6] usb: add usb attached scsi emulation
On Sun, Jul 15, 2012 at 7:55 AM, Deep Debroy ddeb...@gmail.com wrote: On Thu, Jul 12, 2012 at 6:08 AM, Gerd Hoffmann kra...@redhat.com wrote: $subject says all. First cut. It's a pure UAS (usb attached scsi) emulation, without BOT (bulk-only transport) compatibility. If your guest can't handle it use usb-storage instead. The emulation works like any other scsi hba emulation (eps, lsi, virtio, megasas, ...). It provides just the HBA where you can attach scsi devices as you like using '-device'. A single scsi target with up to 256 luns is supported. For now only usb 2.0 transport is supported. This will change in the future though as I plan to use this as playground when codeing up testing usb 3.0 transport and streams support in the qemu usb core and the xhci emulation. No migration support yet. I'm planning to add usb 3.0 support first as this probably requires saving additional state. Special thanks go to Paolo for bringing the qemu scsi emulation into shape, so this can be added nicely without having to touch a single line of scsi code. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- docs/usb-storage.txt | 38 +++ hw/usb/Makefile.objs |1 + hw/usb/dev-uas.c | 779 ++ trace-events | 14 + 4 files changed, 832 insertions(+), 0 deletions(-) create mode 100644 docs/usb-storage.txt create mode 100644 hw/usb/dev-uas.c diff --git a/docs/usb-storage.txt b/docs/usb-storage.txt new file mode 100644 index 000..ff97559 --- /dev/null +++ b/docs/usb-storage.txt @@ -0,0 +1,38 @@ + +qemu usb storage emulation +-- + +Qemu has two emulations for usb storage devices. + +Number one emulates the classic bulk-only transport protocol which is +used by 99% of the usb sticks on the marked today and is called +usb-storage. Usage (hooking up to xhci, other host controllers work +too): + + qemu ${other_vm_args}\ + -drive if=none,id=stick,file=/path/to/file.img \ + -device nec-usb-xhci,id=xhci\ + -device usb-storage,bus=xhci.0,drive=stick + + +Number two is the newer usb attached scsi transport. This one doesn't +automagically create a scsi disk, so you have to explicitly attach one +manually. Multiple logical units are supported. Here is an example +with tree logical units: + + qemu ${other_vm_args}\ + -drive if=none,id=uas-disk1,file=/path/to/file1.img \ + -drive if=none,id=uas-disk2,file=/path/to/file2.img \ + -drive if=none,id=uas-cdrom,media=cdrom,file=/path/to/image.iso \ + -device nec-usb-xhci,id=xhci\ + -device usb-uas,id=uas,bus=xhci.0 \ + -device scsi-hd,bus=uas.0,scsi-id=0,lun=0,drive=uas-disk1 \ + -device scsi-hd,bus=uas.0,scsi-id=0,lun=1,drive=uas-disk2 \ + -device scsi-cd,bus=uas.0,scsi-id=0,lun=5,drive=uas-cdrom + + +enjoy, + Gerd + +-- +Gerd Hoffmann kra...@redhat.com diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs index 9c7ddf5..4225136 100644 --- a/hw/usb/Makefile.objs +++ b/hw/usb/Makefile.objs @@ -11,3 +11,4 @@ common-obj-y += core.o bus.o desc.o dev-hub.o common-obj-y += host-$(HOST_USB).o dev-bluetooth.o common-obj-y += dev-hid.o dev-storage.o dev-wacom.o common-obj-y += dev-serial.o dev-network.o dev-audio.o +common-obj-y += dev-uas.o diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c new file mode 100644 index 000..9b02ff4 --- /dev/null +++ b/hw/usb/dev-uas.c @@ -0,0 +1,779 @@ +/* + * UAS (USB Attached SCSI) emulation + * + * Copyright Red Hat, Inc. 2012 + * + * Author: Gerd Hoffmann kra...@redhat.com + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include qemu-common.h +#include qemu-option.h +#include qemu-config.h +#include trace.h + +#include hw/usb.h +#include hw/usb/desc.h +#include hw/scsi.h +#include hw/scsi-defs.h + +/* - */ + +#define UAS_UI_COMMAND 0x01 +#define UAS_UI_SENSE0x03 +#define UAS_UI_RESPONSE 0x04 +#define UAS_UI_TASK_MGMT0x05 +#define UAS_UI_READ_READY 0x06 +#define UAS_UI_WRITE_READY 0x07 + +#define UAS_RC_TMF_COMPLETE 0x00 +#define UAS_RC_INVALID_INFO_UNIT0x02 +#define UAS_RC_TMF_NOT_SUPPORTED0x04 +#define UAS_RC_TMF_FAILED 0x05 +#define UAS_RC_TMF_SUCCEEDED0x08 +#define UAS_RC_INCORRECT_LUN0x09 +#define UAS_RC_OVERLAPPED_TAG 0x0a + +#define UAS_TMF_ABORT_TASK 0x01 +#define UAS_TMF_ABORT_TASK_SET 0x02 +#define UAS_TMF_CLEAR_TASK_SET 0x04 +#define
[Qemu-devel] [PATCH 11/12] raw-posix: rename raw-posix-aio.h, hide unavailable prototypes
Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- block/{raw-posix-aio.h = raw-aio.h} | 10 ++ block/raw-posix.c|2 +- linux-aio.c |2 +- 3 files changed, 8 insertions(+), 6 deletions(-) rename block/{raw-posix-aio.h = raw-aio.h} (86%) diff --git a/block/raw-posix-aio.h b/block/raw-aio.h similarity index 86% rename from block/raw-posix-aio.h rename to block/raw-aio.h index c714367..b3bb073 100644 --- a/block/raw-posix-aio.h +++ b/block/raw-aio.h @@ -1,5 +1,5 @@ /* - * QEMU Posix block I/O backend AIO support + * Declarations for AIO in the raw protocol * * Copyright IBM, Corp. 2008 * @@ -12,8 +12,8 @@ * Contributions after 2012-01-13 are licensed under the terms of the * GNU GPL, version 2 or (at your option) any later version. */ -#ifndef QEMU_RAW_POSIX_AIO_H -#define QEMU_RAW_POSIX_AIO_H +#ifndef QEMU_RAW_AIO_H +#define QEMU_RAW_AIO_H /* AIO request types */ #define QEMU_AIO_READ 0x0001 @@ -28,9 +28,11 @@ /* linux-aio.c - Linux native implementation */ +#ifdef CONFIG_LINUX_AIO void *laio_init(void); BlockDriverAIOCB *laio_submit(BlockDriverState *bs, void *aio_ctx, int fd, int64_t sector_num, QEMUIOVector *qiov, int nb_sectors, BlockDriverCompletionFunc *cb, void *opaque, int type); +#endif -#endif /* QEMU_RAW_POSIX_AIO_H */ +#endif /* QEMU_RAW_AIO_H */ diff --git a/block/raw-posix.c b/block/raw-posix.c index 3e2e822..64296a6 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -29,7 +29,7 @@ #include module.h #include trace.h #include thread-pool.h -#include block/raw-posix-aio.h +#include block/raw-aio.h #if defined(__APPLE__) (__MACH__) #include paths.h diff --git a/linux-aio.c b/linux-aio.c index 779f793..67c49af 100644 --- a/linux-aio.c +++ b/linux-aio.c @@ -9,7 +9,7 @@ */ #include qemu-common.h #include qemu-aio.h -#include block/raw-posix-aio.h +#include block/raw-aio.h #include event_notifier.h #include libaio.h -- 1.7.10.4
Re: [Qemu-devel] In process NBD server
On Sat, Jul 14, 2012 at 4:52 AM, Eknath Venkataramani eknath.i...@gmail.com wrote: Has the GSoC 2012 project In process NBD server completed? No patches have been posted. I CCed folks I have spoken to about this feature, maybe they have some code. If it hasn't, what would be a good way to pull out (only) files from a windows guest to the host machine, when the guest is in a stopped state? I'm not 100% clear what you are doing. Are you really using the stop QEMU monitor command and then you want to access the disk image? Stefan
[Qemu-devel] [PATCH 03/12] main-loop: use event notifiers
Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- Makefile.objs |4 +-- main-loop.c | 106 - oslib-posix.c | 31 - qemu-common.h |1 - 4 files changed, 17 insertions(+), 125 deletions(-) diff --git a/Makefile.objs b/Makefile.objs index ecdfaf9..6ed1981 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -20,8 +20,8 @@ universal-obj-y += $(qom-obj-y) ### # oslib-obj-y is code depending on the OS (win32 vs posix) oslib-obj-y = osdep.o -oslib-obj-$(CONFIG_WIN32) += oslib-win32.o qemu-thread-win32.o -oslib-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o +oslib-obj-$(CONFIG_WIN32) += oslib-win32.o qemu-thread-win32.o event_notifier-win32.o +oslib-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o event_notifier-posix.o ### # coroutines diff --git a/main-loop.c b/main-loop.c index eb3b6e6..81f49b3 100644 --- a/main-loop.c +++ b/main-loop.c @@ -26,75 +26,12 @@ #include qemu-timer.h #include slirp/slirp.h #include main-loop.h +#include event_notifier.h #ifndef _WIN32 #include compatfd.h -static int io_thread_fd = -1; - -void qemu_notify_event(void) -{ -/* Write 8 bytes to be compatible with eventfd. */ -static const uint64_t val = 1; -ssize_t ret; - -if (io_thread_fd == -1) { -return; -} -do { -ret = write(io_thread_fd, val, sizeof(val)); -} while (ret 0 errno == EINTR); - -/* EAGAIN is fine, a read must be pending. */ -if (ret 0 errno != EAGAIN) { -fprintf(stderr, qemu_notify_event: write() failed: %s\n, -strerror(errno)); -exit(1); -} -} - -static void qemu_event_read(void *opaque) -{ -int fd = (intptr_t)opaque; -ssize_t len; -char buffer[512]; - -/* Drain the notify pipe. For eventfd, only 8 bytes will be read. */ -do { -len = read(fd, buffer, sizeof(buffer)); -} while ((len == -1 errno == EINTR) || len == sizeof(buffer)); -} - -static int qemu_event_init(void) -{ -int err; -int fds[2]; - -err = qemu_eventfd(fds); -if (err == -1) { -return -errno; -} -err = fcntl_setfl(fds[0], O_NONBLOCK); -if (err 0) { -goto fail; -} -err = fcntl_setfl(fds[1], O_NONBLOCK); -if (err 0) { -goto fail; -} -qemu_set_fd_handler2(fds[0], NULL, qemu_event_read, NULL, - (void *)(intptr_t)fds[0]); - -io_thread_fd = fds[1]; -return 0; - -fail: -close(fds[0]); -close(fds[1]); -return err; -} - /* If we have signalfd, we mask out the signals we want to handle and then * use signalfd to listen for them. We rely on whatever the current signal * handler is to dispatch the signals when we receive them. @@ -164,40 +101,22 @@ static int qemu_signal_init(void) #else /* _WIN32 */ -static HANDLE qemu_event_handle = NULL; - -static void dummy_event_handler(void *opaque) -{ -} - -static int qemu_event_init(void) +static int qemu_signal_init(void) { -qemu_event_handle = CreateEvent(NULL, FALSE, FALSE, NULL); -if (!qemu_event_handle) { -fprintf(stderr, Failed CreateEvent: %ld\n, GetLastError()); -return -1; -} -qemu_add_wait_object(qemu_event_handle, dummy_event_handler, NULL); return 0; } +#endif + +static EventNotifier io_thread_notifier; +static int io_thread_initialized; void qemu_notify_event(void) { -if (!qemu_event_handle) { +if (!io_thread_initialized) { return; } -if (!SetEvent(qemu_event_handle)) { -fprintf(stderr, qemu_notify_event: SetEvent failed: %ld\n, -GetLastError()); -exit(1); -} -} - -static int qemu_signal_init(void) -{ -return 0; +event_notifier_set(io_thread_notifier); } -#endif int main_loop_init(void) { @@ -210,11 +129,15 @@ int main_loop_init(void) } /* Note eventfd must be drained before signalfd handlers run */ -ret = qemu_event_init(); +ret = event_notifier_init(io_thread_notifier, 0); if (ret) { return ret; } +io_thread_initialized = true; +event_notifier_set_handler(io_thread_notifier, + (EventNotifierHandler *) + event_notifier_test_and_clear); return 0; } @@ -400,7 +323,8 @@ void qemu_del_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque) void qemu_fd_register(int fd) { -WSAEventSelect(fd, qemu_event_handle, FD_READ | FD_ACCEPT | FD_CLOSE | +WSAEventSelect(fd, event_notifier_get_handle(io_thread_notifier), + FD_READ | FD_ACCEPT | FD_CLOSE | FD_CONNECT | FD_WRITE | FD_OOB); } diff --git a/oslib-posix.c b/oslib-posix.c index 6b7ba64..2c6b044 100644 --- a/oslib-posix.c +++ b/oslib-posix.c @@ -58,9 +58,6 @@ static int
[Qemu-devel] [PATCH 04/12] aio: provide platform-independent API
This adds to aio.c a platform-independent API based on EventNotifiers, that can be used by the portable thread pool. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- aio.c |9 + qemu-aio.h | 19 ++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/aio.c b/aio.c index 0a9eb10..417ceb4 100644 --- a/aio.c +++ b/aio.c @@ -97,6 +97,15 @@ int qemu_aio_set_fd_handler(int fd, return 0; } +void qemu_aio_set_event_notifier(EventNotifier *notifier, + EventNotifierHandler *io_read, + AioFlushEventNotifierHandler *io_flush) +{ +qemu_aio_set_fd_handler(event_notifier_get_fd(notifier), +(IOHandler *)io_read, NULL, +(AioFlushHandler *)io_flush, notifier); +} + void qemu_aio_flush(void) { while (qemu_aio_wait()); diff --git a/qemu-aio.h b/qemu-aio.h index bfdd35f..89c766c 100644 --- a/qemu-aio.h +++ b/qemu-aio.h @@ -16,6 +16,7 @@ #include qemu-common.h #include qemu-char.h +#include event_notifier.h typedef struct BlockDriverAIOCB BlockDriverAIOCB; typedef void BlockDriverCompletionFunc(void *opaque, int ret); @@ -39,7 +40,7 @@ void *qemu_aio_get(AIOPool *pool, BlockDriverState *bs, void qemu_aio_release(void *p); /* Returns 1 if there are still outstanding AIO requests; 0 otherwise */ -typedef int (AioFlushHandler)(void *opaque); +typedef int (AioFlushEventNotifierHandler)(EventNotifier *e); /* Flush any pending AIO operation. This function will block until all * outstanding AIO operations have been completed or cancelled. */ @@ -53,6 +54,10 @@ void qemu_aio_flush(void); * Return whether there is still any pending AIO operation. */ bool qemu_aio_wait(void); +#ifdef CONFIG_POSIX +/* Returns 1 if there are still outstanding AIO requests; 0 otherwise */ +typedef int (AioFlushHandler)(void *opaque); + /* Register a file descriptor and associated callbacks. Behaves very similarly * to qemu_set_fd_handler2. Unlike qemu_set_fd_handler2, these callbacks will * be invoked when using either qemu_aio_wait() or qemu_aio_flush(). @@ -65,5 +70,17 @@ int qemu_aio_set_fd_handler(int fd, IOHandler *io_write, AioFlushHandler *io_flush, void *opaque); +#endif + +/* Register an event notifier and associated callbacks. Behaves very similarly + * to event_notifier_set_handler. Unlike event_notifier_set_handler, these callbacks + * will be invoked when using either qemu_aio_wait() or qemu_aio_flush(). + * + * Code that invokes AIO completion functions should rely on this function + * instead of event_notifier_set_handler. + */ +void qemu_aio_set_event_notifier(EventNotifier *notifier, + EventNotifierHandler *io_read, + AioFlushEventNotifierHandler *io_flush); #endif -- 1.7.10.4
[Qemu-devel] [PATCH 06/12] linux-aio: use event notifiers
Since linux-aio already uses an eventfd, converting it to use the EventNotifier-based API simplifies the code even though it is not meant to be portable. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- linux-aio.c | 49 +++-- 1 file changed, 19 insertions(+), 30 deletions(-) diff --git a/linux-aio.c b/linux-aio.c index fa0fbf3..779f793 100644 --- a/linux-aio.c +++ b/linux-aio.c @@ -10,8 +10,8 @@ #include qemu-common.h #include qemu-aio.h #include block/raw-posix-aio.h +#include event_notifier.h -#include sys/eventfd.h #include libaio.h /* @@ -37,7 +37,7 @@ struct qemu_laiocb { struct qemu_laio_state { io_context_t ctx; -int efd; +EventNotifier e; int count; }; @@ -76,29 +76,17 @@ static void qemu_laio_process_completion(struct qemu_laio_state *s, qemu_aio_release(laiocb); } -static void qemu_laio_completion_cb(void *opaque) +static void qemu_laio_completion_cb(EventNotifier *e) { -struct qemu_laio_state *s = opaque; +struct qemu_laio_state *s = container_of(e, struct qemu_laio_state, e); -while (1) { +while (event_notifier_test_and_clear(s-e)) { struct io_event events[MAX_EVENTS]; -uint64_t val; -ssize_t ret; struct timespec ts = { 0 }; int nevents, i; do { -ret = read(s-efd, val, sizeof(val)); -} while (ret == -1 errno == EINTR); - -if (ret == -1 errno == EAGAIN) -break; - -if (ret != 8) -break; - -do { -nevents = io_getevents(s-ctx, val, MAX_EVENTS, events, ts); +nevents = io_getevents(s-ctx, MAX_EVENTS, MAX_EVENTS, events, ts); } while (nevents == -EINTR); for (i = 0; i nevents; i++) { @@ -112,9 +100,9 @@ static void qemu_laio_completion_cb(void *opaque) } } -static int qemu_laio_flush_cb(void *opaque) +static int qemu_laio_flush_cb(EventNotifier *e) { -struct qemu_laio_state *s = opaque; +struct qemu_laio_state *s = container_of(e, struct qemu_laio_state, e); return (s-count 0) ? 1 : 0; } @@ -146,8 +134,9 @@ static void laio_cancel(BlockDriverAIOCB *blockacb) * We might be able to do this slightly more optimal by removing the * O_NONBLOCK flag. */ -while (laiocb-ret == -EINPROGRESS) -qemu_laio_completion_cb(laiocb-ctx); +while (laiocb-ret == -EINPROGRESS) { +qemu_laio_completion_cb(laiocb-ctx-e); +} } static AIOPool laio_pool = { @@ -186,7 +175,7 @@ BlockDriverAIOCB *laio_submit(BlockDriverState *bs, void *aio_ctx, int fd, __func__, type); goto out_free_aiocb; } -io_set_eventfd(laiocb-iocb, s-efd); +io_set_eventfd(laiocb-iocb, event_notifier_get_fd(s-e)); s-count++; if (io_submit(s-ctx, 1, iocbs) 0) @@ -205,21 +194,21 @@ void *laio_init(void) struct qemu_laio_state *s; s = g_malloc0(sizeof(*s)); -s-efd = eventfd(0, 0); -if (s-efd == -1) +if (event_notifier_init(s-e, false) 0) { goto out_free_state; -fcntl(s-efd, F_SETFL, O_NONBLOCK); +} -if (io_setup(MAX_EVENTS, s-ctx) != 0) +if (io_setup(MAX_EVENTS, s-ctx) != 0) { goto out_close_efd; +} -qemu_aio_set_fd_handler(s-efd, qemu_laio_completion_cb, NULL, -qemu_laio_flush_cb, s); +qemu_aio_set_event_notifier(s-e, qemu_laio_completion_cb, +qemu_laio_flush_cb); return s; out_close_efd: -close(s-efd); +event_notifier_cleanup(s-e); out_free_state: g_free(s); return NULL; -- 1.7.10.4
[Qemu-devel] [PATCH 05/12] aio: add Win32 implementation
The Win32 implementation only accepts EventNotifiers, thus a few drivers are disabled under Windows. It is possible to use the same techniques in main-loop.c and reenable them; alternatively, the drivers can be changed to use threads instead of non-blocking I/O. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- Makefile.objs|6 +- aio.c = aio-posix.c |0 aio-win32.c | 177 ++ block/Makefile.objs |6 +- 4 files changed, 185 insertions(+), 4 deletions(-) rename aio.c = aio-posix.c (100%) create mode 100644 aio-win32.c diff --git a/Makefile.objs b/Makefile.objs index 6ed1981..96d0e68 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -42,11 +42,11 @@ coroutine-obj-$(CONFIG_WIN32) += coroutine-win32.o # block-obj-y is code used by both qemu system emulation and qemu-img block-obj-y = cutils.o cache-utils.o qemu-option.o module.o async.o -block-obj-y += nbd.o block.o aio.o aes.o qemu-config.o qemu-progress.o qemu-sockets.o +block-obj-y += nbd.o block.o aes.o qemu-config.o qemu-progress.o qemu-sockets.o block-obj-y += $(coroutine-obj-y) $(qobject-obj-y) $(version-obj-y) block-obj-$(CONFIG_POSIX) += posix-aio-compat.o -block-obj-$(CONFIG_POSIX) += event_notifier-posix.o -block-obj-$(CONFIG_WIN32) += event_notifier-win32.o +block-obj-$(CONFIG_POSIX) += event_notifier-posix.o aio-posix.o +block-obj-$(CONFIG_WIN32) += event_notifier-win32.o aio-win32.o block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o block-obj-y += block/ diff --git a/aio.c b/aio-posix.c similarity index 100% rename from aio.c rename to aio-posix.c diff --git a/aio-win32.c b/aio-win32.c new file mode 100644 index 000..0936f7f --- /dev/null +++ b/aio-win32.c @@ -0,0 +1,177 @@ +/* + * QEMU aio implementation for Win32 + * + * Copyright IBM Corp., 2008 + * Copyright Red Hat Inc., 2012 + * + * Authors: + * Anthony Liguori aligu...@us.ibm.com + * Paolo Bonzini pbonz...@redhat.com + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + * + * Contributions after 2012-01-13 are licensed under the terms of the + * GNU GPL, version 2 or (at your option) any later version. + */ + +#include qemu-common.h +#include block.h +#include qemu-queue.h +#include qemu_socket.h + +typedef struct AioHandler AioHandler; + +/* The list of registered AIO handlers */ +static QLIST_HEAD(, AioHandler) aio_handlers; + +/* This is a simple lock used to protect the aio_handlers list. Specifically, + * it's used to ensure that no callbacks are removed while we're walking and + * dispatching callbacks. + */ +static int walking_handlers; + +struct AioHandler +{ +EventNotifier *e; +EventNotifierHandler *io_notify; +AioFlushEventNotifierHandler *io_flush; +int deleted; +QLIST_ENTRY(AioHandler) node; +}; + +void qemu_aio_set_event_notifier(EventNotifier *e, + EventNotifierHandler *io_notify, + AioFlushEventNotifierHandler *io_flush) +{ +AioHandler *node; + +QLIST_FOREACH(node, aio_handlers, node) { +if (node-e == e !node-deleted) { +break; +} +} + +/* Are we deleting the fd handler? */ +if (!io_notify) { +if (node) { +qemu_del_wait_object(event_notifier_get_handle(e), + (WaitObjectFunc *) node-io_notify, e); + +/* If the lock is held, just mark the node as deleted */ +if (walking_handlers) { +node-deleted = 1; +} else { +/* Otherwise, delete it for real. We can't just mark it as + * deleted because deleted nodes are only cleaned up after + * releasing the walking_handlers lock. + */ +QLIST_REMOVE(node, node); +g_free(node); +} +} +} else { +if (node == NULL) { +/* Alloc and insert if it's not already there */ +node = g_malloc0(sizeof(AioHandler)); +node-e = e; +QLIST_INSERT_HEAD(aio_handlers, node, node); +} +/* Update handler with latest information */ +node-io_notify = io_notify; +node-io_flush = io_flush; +qemu_add_wait_object(event_notifier_get_handle(e), + (WaitObjectFunc *) io_notify, e); +} +} + +void qemu_aio_flush(void) +{ +while (qemu_aio_wait()); +} + +bool qemu_aio_wait(void) +{ +AioHandler *node; +HANDLE events[MAXIMUM_WAIT_OBJECTS + 1]; +bool busy; +int count; +int ret; +int timeout; + +/* + * If there are callbacks left that have been queued, we need to call then. + * Do not call select in this case, because it is possible that the caller + * does not need a complete flush (as is the case for qemu_aio_wait loops). + */ +if (qemu_bh_poll()) { +return true; +
Re: [Qemu-devel] [PATCH] qemu-io: Fix memory leaks
On Thu, Jul 12, 2012 at 2:02 PM, Kevin Wolf kw...@redhat.com wrote: Almost all callers of create_iovec() forgot to destroy the qiov when the request has completed. Signed-off-by: Kevin Wolf kw...@redhat.com --- qemu-io.c |4 1 files changed, 4 insertions(+), 0 deletions(-) Reviewed-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
[Qemu-devel] [PATCH 07/12] qemu-thread: add QemuSemaphore
The new thread pool will use semaphores instead of condition variables, because QemuCond does not have qemu_cond_timedwait. (I also like it more this way, since semaphores model well the producer-consumer problem). Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- qemu-thread-posix.c | 74 +++ qemu-thread-posix.h |5 qemu-thread-win32.c | 35 qemu-thread-win32.h |4 +++ qemu-thread.h |7 + 5 files changed, 125 insertions(+) diff --git a/qemu-thread-posix.c b/qemu-thread-posix.c index 9e1b5fb..251fef0 100644 --- a/qemu-thread-posix.c +++ b/qemu-thread-posix.c @@ -17,6 +17,9 @@ #include signal.h #include stdint.h #include string.h +#include limits.h +#include unistd.h +#include sys/time.h #include qemu-thread.h static void error_exit(int err, const char *msg) @@ -115,6 +118,77 @@ void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex) error_exit(err, __func__); } +void qemu_sem_init(QemuSemaphore *sem, int init) +{ +int rc; + +rc = sem_init(sem-sem, 0, init); +if (rc 0) { +error_exit(errno, __func__); +} +} + +void qemu_sem_destroy(QemuSemaphore *sem) +{ +int rc; + +rc = sem_destroy(sem-sem); +if (rc 0) { +error_exit(errno, __func__); +} +} + +void qemu_sem_post(QemuSemaphore *sem) +{ +int rc; + +rc = sem_post(sem-sem); +if (rc 0) { +error_exit(errno, __func__); +} +} + +int qemu_sem_timedwait(QemuSemaphore *sem, int ms) +{ +int rc; + +if (ms = 0) { +/* This is cheaper than sem_timedwait. */ +rc = sem_trywait(sem-sem); +if (rc == -1 errno == EAGAIN) { +return -1; +} +} else { +struct timeval tv; +struct timespec ts; +gettimeofday(tv, NULL); +ts.tv_nsec = tv.tv_usec * 1000 + (ms % 1000) * 100; +ts.tv_sec = tv.tv_sec + ms / 1000; +if (ts.tv_nsec = 10) { +ts.tv_sec++; +ts.tv_nsec -= 10; +} +rc = sem_timedwait(sem-sem, ts); +if (rc == -1 errno == ETIMEDOUT) { +return -1; +} +} +if (rc 0) { +error_exit(errno, __func__); +} +return 0; +} + +void qemu_sem_wait(QemuSemaphore *sem) +{ +int rc; + +rc = sem_wait(sem-sem); +if (rc 0) { +error_exit(errno, __func__); +} +} + void qemu_thread_create(QemuThread *thread, void *(*start_routine)(void*), void *arg, int mode) diff --git a/qemu-thread-posix.h b/qemu-thread-posix.h index ee4618e..2542c15 100644 --- a/qemu-thread-posix.h +++ b/qemu-thread-posix.h @@ -1,6 +1,7 @@ #ifndef __QEMU_THREAD_POSIX_H #define __QEMU_THREAD_POSIX_H 1 #include pthread.h +#include semaphore.h struct QemuMutex { pthread_mutex_t lock; @@ -10,6 +11,10 @@ struct QemuCond { pthread_cond_t cond; }; +struct QemuSemaphore { +sem_t sem; +}; + struct QemuThread { pthread_t thread; }; diff --git a/qemu-thread-win32.c b/qemu-thread-win32.c index 3524c8b..78602d2 100644 --- a/qemu-thread-win32.c +++ b/qemu-thread-win32.c @@ -192,6 +192,41 @@ void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex) qemu_mutex_lock(mutex); } +void qemu_sem_init(QemuSemaphore *sem, int init) +{ +/* Manual reset. */ +sem-sema = CreateSemaphore(NULL, init, LONG_MAX, NULL); +} + +void qemu_sem_destroy(QemuSemaphore *sem) +{ +CloseHandle(sem-sema); +} + +void qemu_sem_post(QemuSemaphore *sem) +{ +ReleaseSemaphore(sem-sema, 1, NULL); +} + +int qemu_sem_timedwait(QemuSemaphore *sem, int ms) +{ +int rc = WaitForSingleObject(sem-sema, ms); +if (rc == WAIT_OBJECT_0) { +return 0; +} +if (rc != WAIT_TIMEOUT) { +error_exit(GetLastError(), __func__); +} +return -1; +} + +void qemu_sem_wait(QemuSemaphore *sem) +{ +if (WaitForSingleObject(sem-sema, INFINITE) != WAIT_OBJECT_0) { +error_exit(GetLastError(), __func__); +} +} + struct QemuThreadData { /* Passed to win32_start_routine. */ void *(*start_routine)(void *); diff --git a/qemu-thread-win32.h b/qemu-thread-win32.h index b9d1be8..13adb95 100644 --- a/qemu-thread-win32.h +++ b/qemu-thread-win32.h @@ -13,6 +13,10 @@ struct QemuCond { HANDLE continue_event; }; +struct QemuSemaphore { +HANDLE sema; +}; + typedef struct QemuThreadData QemuThreadData; struct QemuThread { QemuThreadData *data; diff --git a/qemu-thread.h b/qemu-thread.h index a78a8f2..c3f960e 100644 --- a/qemu-thread.h +++ b/qemu-thread.h @@ -5,6 +5,7 @@ typedef struct QemuMutex QemuMutex; typedef struct QemuCond QemuCond; +typedef struct QemuSemaphore QemuSemaphore; typedef struct QemuThread QemuThread; #ifdef _WIN32 @@ -37,6 +38,12 @@ void qemu_cond_signal(QemuCond *cond); void qemu_cond_broadcast(QemuCond *cond); void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex); +void
[Qemu-devel] [PATCH 02/12] event_notifier: add Win32 implementation
The Win32 implementation of EventNotifier is a trivial wrapper around manual-reset events. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- Makefile.objs |4 +- event_notifier.c = event_notifier-posix.c |0 event_notifier-win32.c | 59 event_notifier.h | 17 +++- 4 files changed, 77 insertions(+), 3 deletions(-) rename event_notifier.c = event_notifier-posix.c (100%) create mode 100644 event_notifier-win32.c diff --git a/Makefile.objs b/Makefile.objs index 625c4d5..ecdfaf9 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -45,6 +45,8 @@ block-obj-y = cutils.o cache-utils.o qemu-option.o module.o async.o block-obj-y += nbd.o block.o aio.o aes.o qemu-config.o qemu-progress.o qemu-sockets.o block-obj-y += $(coroutine-obj-y) $(qobject-obj-y) $(version-obj-y) block-obj-$(CONFIG_POSIX) += posix-aio-compat.o +block-obj-$(CONFIG_POSIX) += event_notifier-posix.o +block-obj-$(CONFIG_WIN32) += event_notifier-win32.o block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o block-obj-y += block/ @@ -90,7 +92,7 @@ common-obj-y += bt-host.o bt-vhci.o common-obj-y += iov.o acl.o common-obj-$(CONFIG_POSIX) += compatfd.o -common-obj-y += notify.o event_notifier.o +common-obj-y += notify.o common-obj-y += qemu-timer.o qemu-timer-common.o common-obj-$(CONFIG_SLIRP) += slirp/ diff --git a/event_notifier.c b/event_notifier-posix.c similarity index 100% rename from event_notifier.c rename to event_notifier-posix.c diff --git a/event_notifier-win32.c b/event_notifier-win32.c new file mode 100644 index 000..c723dad --- /dev/null +++ b/event_notifier-win32.c @@ -0,0 +1,59 @@ +/* + * event notifier support for Win32 + * + * Copyright Red Hat, Inc. 2012 + * + * Authors: + * Paolo Bonzini pbonz...@redhat.com + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include qemu-common.h +#include event_notifier.h +#include main-loop.h + +int event_notifier_init(EventNotifier *e, int active) +{ +e-event = CreateEvent(NULL, FALSE, FALSE, NULL); +assert(e-event); +return 0; +} + +void event_notifier_cleanup(EventNotifier *e) +{ +CloseHandle(e-event); +} + +HANDLE event_notifier_get_handle(EventNotifier *e) +{ +return e-event; +} + +int event_notifier_set_handler(EventNotifier *e, + EventNotifierHandler *handler) +{ +if (handler) { +return qemu_add_wait_object(e-event, (IOHandler *)handler, e); +} else { +qemu_del_wait_object(e-event, (IOHandler *)handler, e); +return 0; +} +} + +int event_notifier_set(EventNotifier *e) +{ +SetEvent(e-event); +return 0; +} + +int event_notifier_test_and_clear(EventNotifier *e) +{ +int ret = WaitForSingleObject(e-event, 0); +if (ret == WAIT_OBJECT_0) { +ResetEvent(e-event); +return true; +} +return false; +} diff --git a/event_notifier.h b/event_notifier.h index f04d12d..88b57af 100644 --- a/event_notifier.h +++ b/event_notifier.h @@ -15,19 +15,32 @@ #include qemu-common.h +#ifdef _WIN32 +#include windows.h +#endif + struct EventNotifier { +#ifdef _WIN32 +HANDLE event; +#else int rfd; int wfd; +#endif }; typedef void EventNotifierHandler(EventNotifier *); -void event_notifier_init_fd(EventNotifier *, int fd); int event_notifier_init(EventNotifier *, int active); void event_notifier_cleanup(EventNotifier *); -int event_notifier_get_fd(EventNotifier *); int event_notifier_set(EventNotifier *); int event_notifier_test_and_clear(EventNotifier *); int event_notifier_set_handler(EventNotifier *, EventNotifierHandler *); +#ifdef CONFIG_POSIX +void event_notifier_init_fd(EventNotifier *, int fd); +int event_notifier_get_fd(EventNotifier *); +#else +HANDLE event_notifier_get_handle(EventNotifier *); +#endif + #endif -- 1.7.10.4
Re: [Qemu-devel] [PATCH 07/12] qemu-thread: add QemuSemaphore
On 2012-07-16 12:42, Paolo Bonzini wrote: The new thread pool will use semaphores instead of condition variables, because QemuCond does not have qemu_cond_timedwait. I'll post an updated patch (according to last round's review comments) that adds this service for POSIX. I bet you'll find a way to extend it to Win32 if that is required. ;) (I also like it more this way, since semaphores model well the producer-consumer problem). Let's not introduce another synchronization mechanism unless there is a real need. Semaphores tend to be misused for things they don't fit, so better keep them out of reach. Also, if you do producer-consumer this way, you need a down() for every entity you dequeue. In contrast, you only interact with condition variables if there the consumer queue is empty - less atomic ops. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
[Qemu-devel] [PATCH] qemu-thread: Introduce qemu_cond_timedwait for POSIX
First user will be POSIX compat aio. Windows use cases aren't in sight, so this remains a POSIX-only service for now. This version uses CLOCK_MONOTONIC for the timeout to avoid jumps on wall clock adjustments, provided the host support pthread_condattr_setclock. Signed-off-by: Jan Kiszka jan.kis...@siemens.com --- configure | 21 + qemu-thread-posix.c | 46 +- qemu-thread-posix.h |5 + 3 files changed, 71 insertions(+), 1 deletions(-) diff --git a/configure b/configure index 0a3896e..dec39c8 100755 --- a/configure +++ b/configure @@ -2135,6 +2135,23 @@ if test $mingw32 != yes -a $pthread = no; then fi ## +# pthread_condattr_setclock probe +condattr_setclock=no +if test pthread != no ; then + cat $TMPC EOF +#include pthread.h +int main(void) { + pthread_condattr_t attr; + pthread_condattr_init(attr); + return pthread_condattr_setclock(attr, CLOCK_MONOTONIC); +} +EOF + if compile_prog $LIBS $TMPE; then +condattr_setclock=yes + fi +fi + +## # rbd probe if test $rbd != no ; then cat $TMPC EOF @@ -3040,6 +3057,7 @@ echo preadv support$preadv echo fdatasync $fdatasync echo madvise $madvise echo posix_madvise $posix_madvise +echo condattr_setclock $condattr_setclock echo uuid support $uuid echo libcap-ng support $cap_ng echo vhost-net support $vhost_net @@ -3324,6 +3342,9 @@ fi if test $posix_madvise = yes ; then echo CONFIG_POSIX_MADVISE=y $config_host_mak fi +if test $condattr_setclock = yes ; then + echo CONFIG_CONDATTR_SETCLOCK=y $config_host_mak +fi if test $spice = yes ; then echo CONFIG_SPICE=y $config_host_mak diff --git a/qemu-thread-posix.c b/qemu-thread-posix.c index 9e1b5fb..ed6ab06 100644 --- a/qemu-thread-posix.c +++ b/qemu-thread-posix.c @@ -17,6 +17,8 @@ #include signal.h #include stdint.h #include string.h +#include sys/time.h +#include config-host.h #include qemu-thread.h static void error_exit(int err, const char *msg) @@ -73,8 +75,20 @@ void qemu_mutex_unlock(QemuMutex *mutex) void qemu_cond_init(QemuCond *cond) { int err; +pthread_condattr_t attr; -err = pthread_cond_init(cond-cond, NULL); +err = pthread_condattr_init(attr); +if (err) { +error_exit(err, __func__); +} +#ifdef CONFIG_CONDATTR_SETCLOCK +err = pthread_condattr_setclock(attr, CLOCK_MONOTONIC); +if (err) { +error_exit(err, __func__); +} +#endif + +err = pthread_cond_init(cond-cond, attr); if (err) error_exit(err, __func__); } @@ -115,6 +129,36 @@ void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex) error_exit(err, __func__); } +/* Returns true if condition was signals, false if timed out. */ +bool qemu_cond_timedwait(QemuCond *cond, QemuMutex *mutex, + unsigned int timeout_ms) +{ +struct timespec ts; +int err; + +#ifdef CONFIG_CONDATTR_SETCLOCK +clock_gettime(CLOCK_MONOTONIC, ts); +#else +struct timeval tv; + +gettimeofday(tv, NULL); +ts.tv_sec = tv.tv_sec; +ts.tv_nsec = tv.tv_usec * 1000; +#endif + +ts.tv_sec += timeout_ms / 1000; +ts.tv_nsec += (timeout_ms % 1000) * 100; +if (ts.tv_nsec 10) { +ts.tv_sec++; +ts.tv_nsec -= 10; +} +err = pthread_cond_timedwait(cond-cond, mutex-lock, ts); +if (err err != ETIMEDOUT) { +error_exit(err, __func__); +} +return err == 0; +} + void qemu_thread_create(QemuThread *thread, void *(*start_routine)(void*), void *arg, int mode) diff --git a/qemu-thread-posix.h b/qemu-thread-posix.h index ee4618e..9f00524 100644 --- a/qemu-thread-posix.h +++ b/qemu-thread-posix.h @@ -1,5 +1,6 @@ #ifndef __QEMU_THREAD_POSIX_H #define __QEMU_THREAD_POSIX_H 1 +#include stdbool.h #include pthread.h struct QemuMutex { @@ -14,4 +15,8 @@ struct QemuThread { pthread_t thread; }; +/* only provided for posix so far */ +bool qemu_cond_timedwait(QemuCond *cond, QemuMutex *mutex, + unsigned int timeout_ms); + #endif -- 1.7.3.4
[Qemu-devel] [PATCH v2 2/2] fdc-test: introduce test_relative_seek
Signed-off-by: Pavel Hrdina phrd...@redhat.com --- tests/fdc-test.c | 46 - 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/tests/fdc-test.c b/tests/fdc-test.c index 585fb0e..4bc3515 100644 --- a/tests/fdc-test.c +++ b/tests/fdc-test.c @@ -47,9 +47,11 @@ enum { }; enum { -CMD_SENSE_INT = 0x08, -CMD_SEEK= 0x0f, -CMD_READ= 0xe6, +CMD_SENSE_INT = 0x08, +CMD_SEEK= 0x0f, +CMD_READ= 0xe6, +CMD_RELATIVE_SEEK_OUT = 0x8f, +CMD_RELATIVE_SEEK_IN= 0xcf, }; enum { @@ -91,13 +93,17 @@ static uint8_t floppy_recv(void) return inb(FLOPPY_BASE + reg_fifo); } -static void ack_irq(void) +static uint8_t ack_irq(void) { +uint8_t ret; + g_assert(get_irq(FLOPPY_IRQ)); floppy_send(CMD_SENSE_INT); floppy_recv(); -floppy_recv(); +ret = floppy_recv(); g_assert(!get_irq(FLOPPY_IRQ)); + +return ret; } static uint8_t send_read_command(void) @@ -281,6 +287,35 @@ static void test_sense_interrupt(void) floppy_recv(); } +static void test_relative_seek(void) +{ +uint8_t drive = 0; +uint8_t head = 0; +uint8_t cyl = 1; +uint8_t ret; + +/* Send seek to track 0 */ +send_step_pulse(0); + +/* Send relative seek to increase track by 1 */ +floppy_send(CMD_RELATIVE_SEEK_OUT); +floppy_send(head 2 | drive); +g_assert(!get_irq(FLOPPY_IRQ)); +floppy_send(cyl); + +ret = ack_irq(); +g_assert(ret == 1); + +/* Send relative seek to decrease track by 1 */ +floppy_send(CMD_RELATIVE_SEEK_IN); +floppy_send(head 2 | drive); +g_assert(!get_irq(FLOPPY_IRQ)); +floppy_send(cyl); + +ret = ack_irq(); +g_assert(ret == 0); +} + /* success if no crash or abort */ static void fuzz_registers(void) { @@ -329,6 +371,7 @@ int main(int argc, char **argv) qtest_add_func(/fdc/read_without_media, test_read_without_media); qtest_add_func(/fdc/media_change, test_media_change); qtest_add_func(/fdc/sense_interrupt, test_sense_interrupt); +qtest_add_func(/fdc/relative_seek, test_relative_seek); qtest_add_func(/fdc/fuzz-registers, fuzz_registers); ret = g_test_run(); -- 1.7.10.4
[Qemu-devel] [PATCH v2 1/2] fdc: fix relative seek
Signed-off-by: Pavel Hrdina phrd...@redhat.com --- hw/fdc.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/fdc.c b/hw/fdc.c index edf0706..decb1f7 100644 --- a/hw/fdc.c +++ b/hw/fdc.c @@ -1705,7 +1705,8 @@ static void fdctrl_handle_relative_seek_out(FDCtrl *fdctrl, int direction) fd_seek(cur_drv, cur_drv-head, cur_drv-max_track - 1, cur_drv-sect, 1); } else { -fd_seek(cur_drv, cur_drv-head, fdctrl-fifo[2], cur_drv-sect, 1); +fd_seek(cur_drv, cur_drv-head, +cur_drv-track + fdctrl-fifo[2], cur_drv-sect, 1); } fdctrl_reset_fifo(fdctrl); /* Raise Interrupt */ @@ -1721,7 +1722,8 @@ static void fdctrl_handle_relative_seek_in(FDCtrl *fdctrl, int direction) if (fdctrl-fifo[2] cur_drv-track) { fd_seek(cur_drv, cur_drv-head, 0, cur_drv-sect, 1); } else { -fd_seek(cur_drv, cur_drv-head, fdctrl-fifo[2], cur_drv-sect, 1); +fd_seek(cur_drv, cur_drv-head, +cur_drv-track - fdctrl-fifo[2], cur_drv-sect, 1); } fdctrl_reset_fifo(fdctrl); /* Raise Interrupt */ -- 1.7.10.4
Re: [Qemu-devel] [PATCH 0/3] Introduce virtqueue_get_avail_bytes()
On (Fri) 06 Jul 2012 [16:07:06], Amit Shah wrote: The current virtqueue_avail_bytes() is a weird API: it's oddly-named: doesn't tell us what the API is going to do, and also suits just one use-case (that in virtio-net.c). Introduce virtqueue_get_avail_bytes(), which returns the number of bytes in the vq available for input as well as output. virtqueue_avail_bytes() is made a wrapper around this new function for now. It should be deprecated soon, though. Doing this will also help with the virtio-rng patch where a VirtQueueElement is popped only to find out what its size is. With this series applied, the popping (and the subsequent save/load of state for migration) isn't necessary. The virtio-serial-bus code becomes better too, that's patch 3 here. Ping? Amit
[Qemu-devel] [PATCH] qemu-options.hx: Fix set_password and expire_password description
From: Michal Novotny minov...@redhat.com The description for set_password and expire_password commands is incomplete. This patch fixes the man page that is being generated to match the real behaviour of these functions. Signed-off-by: Michal Novotny minov...@redhat.com --- qemu-options.hx | 17 +++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index ecf7ca1..679e3ca 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1030,8 +1030,21 @@ is a TCP port number, not a display number. @item password Require that password based authentication is used for client connections. -The password must be set separately using the @code{change} command in the -@ref{pcsys_monitor} + +The password must be set separately using the @code{set_password} command in +the @ref{pcsys_monitor}. The syntax to change your password is: +@code{set_password protocol password} where protocol could be either +vnc or spice. + +If you would like to change protocol password expiration, you should use +@code{expire_password protocol expiration-time} where expiration time could +be one of the following options: now, never, +seconds or UNIX time of +expiration, e.g. +60 to make password expire in 60 seconds, or 1335196800 +to make password expire on Mon Apr 23 12:00:00 EDT 2012 (UNIX time for this +date and time). + +You can also use keywords now or never for the expiration time to +allow protocol password to expire immediately or never expire. @item tls -- 1.7.10.4
Re: [Qemu-devel] [PATCH 07/12] qemu-thread: add QemuSemaphore
Il 16/07/2012 14:00, Jan Kiszka ha scritto: On 2012-07-16 12:42, Paolo Bonzini wrote: The new thread pool will use semaphores instead of condition variables, because QemuCond does not have qemu_cond_timedwait. I'll post an updated patch (according to last round's review comments) that adds this service for POSIX. I bet you'll find a way to extend it to Win32 if that is required. ;) I can do that (or just use pthreads-win32), but only at the cost of making cond_wait() slower and more complex. (I also like it more this way, since semaphores model well the producer-consumer problem). Let's not introduce another synchronization mechanism unless there is a real need. Semaphores tend to be misused for things they don't fit, so better keep them out of reach. That's what patch review is for... Also, if you do producer-consumer this way, you need a down() for every entity you dequeue. In contrast, you only interact with condition variables if there the consumer queue is empty - less atomic ops. It doesn't really matter. You want the thread pool to service requests as fast as possible, which means you'll have always at least one free thread waiting on the semaphore or cv. So, with either semaphores or cvs, the slow path is actually the normal case. Paolo
Re: [Qemu-devel] [PATCH v2 1/2] fdc: fix relative seek
Am 16.07.2012 14:25, schrieb Pavel Hrdina: Signed-off-by: Pavel Hrdina phrd...@redhat.com --- hw/fdc.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) I applied both to the block branch for now. This restores the behaviour as it was before 6be01b1e. However, I believe it is still wrong: The direction should be interpreted the other way round, i.e. seek_out should decrease the cylinder number and seek_in should increase it. Do you have a guest that actually uses this command? Kevin
[Qemu-devel] [PATCH v2 4/6] device_tree: Add support for reading device tree properties
Add support for reading device tree properties (both generic and single-cell ones) to QEMU's convenience wrapper layer. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- device_tree.c | 30 ++ device_tree.h |4 2 files changed, 34 insertions(+), 0 deletions(-) diff --git a/device_tree.c b/device_tree.c index b366fdd..d7a9b6b 100644 --- a/device_tree.c +++ b/device_tree.c @@ -178,6 +178,36 @@ int qemu_devtree_setprop_string(void *fdt, const char *node_path, return r; } +const void *qemu_devtree_getprop(void *fdt, const char *node_path, + const char *property, int *lenp) +{ +int len; +const void *r; +if (!lenp) { +lenp = len; +} +r = fdt_getprop(fdt, findnode_nofail(fdt, node_path), property, lenp); +if (!r) { +fprintf(stderr, %s: Couldn't get %s/%s: %s\n, __func__, +node_path, property, fdt_strerror(*lenp)); +exit(1); +} +return r; +} + +uint32_t qemu_devtree_getprop_cell(void *fdt, const char *node_path, + const char *property) +{ +int len; +const uint32_t *p = qemu_devtree_getprop(fdt, node_path, property, len); +if (len != 4) { +fprintf(stderr, %s: %s/%s not 4 bytes long (not a cell?)\n, +__func__, node_path, property); +exit(1); +} +return be32_to_cpu(*p); +} + uint32_t qemu_devtree_get_phandle(void *fdt, const char *path) { uint32_t r; diff --git a/device_tree.h b/device_tree.h index 2244270..f7a3e6c 100644 --- a/device_tree.h +++ b/device_tree.h @@ -28,6 +28,10 @@ int qemu_devtree_setprop_string(void *fdt, const char *node_path, int qemu_devtree_setprop_phandle(void *fdt, const char *node_path, const char *property, const char *target_node_path); +const void *qemu_devtree_getprop(void *fdt, const char *node_path, + const char *property, int *lenp); +uint32_t qemu_devtree_getprop_cell(void *fdt, const char *node_path, + const char *property); uint32_t qemu_devtree_get_phandle(void *fdt, const char *path); uint32_t qemu_devtree_alloc_phandle(void *fdt); int qemu_devtree_nop_node(void *fdt, const char *node_path); -- 1.7.5.4
[Qemu-devel] [PATCH v2 5/6] hw/arm_boot.c: Support DTBs which use 64 bit addresses
Support the case where the device tree blob specifies that #address-cells and #size-cells are greater than 1. (This is needed for device trees which can handle 64 bit physical addresses and thus total RAM sizes over 4GB.) Signed-off-by: Peter Maydell peter.mayd...@linaro.org Reviewed-by: Peter A. G. Crosthwaite peter.crosthwa...@petalogix.com --- hw/arm_boot.c | 35 --- 1 files changed, 32 insertions(+), 3 deletions(-) diff --git a/hw/arm_boot.c b/hw/arm_boot.c index af71ed6..a6e9143 100644 --- a/hw/arm_boot.c +++ b/hw/arm_boot.c @@ -216,11 +216,12 @@ static void set_kernel_args_old(const struct arm_boot_info *info) static int load_dtb(target_phys_addr_t addr, const struct arm_boot_info *binfo) { #ifdef CONFIG_FDT -uint32_t mem_reg_property[] = { cpu_to_be32(binfo-loader_start), -cpu_to_be32(binfo-ram_size) }; +uint32_t *mem_reg_property; +uint32_t mem_reg_propsize; void *fdt = NULL; char *filename; int size, rc; +uint32_t acells, scells, hival; filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, binfo-dtb_filename); if (!filename) { @@ -236,8 +237,36 @@ static int load_dtb(target_phys_addr_t addr, const struct arm_boot_info *binfo) } g_free(filename); +acells = qemu_devtree_getprop_cell(fdt, /, #address-cells); +scells = qemu_devtree_getprop_cell(fdt, /, #size-cells); +if (acells == 0 || scells == 0) { +fprintf(stderr, dtb file invalid (#address-cells or #size-cells 0)\n); +return -1; +} + +mem_reg_propsize = acells + scells; +mem_reg_property = g_new0(uint32_t, mem_reg_propsize); +mem_reg_property[acells - 1] = cpu_to_be32(binfo-loader_start); +hival = cpu_to_be32(binfo-loader_start 32); +if (acells 1) { +mem_reg_property[acells - 2] = hival; +} else if (hival != 0) { +fprintf(stderr, qemu: dtb file not compatible with +RAM start address 4GB\n); +exit(1); +} +mem_reg_property[acells + scells - 1] = cpu_to_be32(binfo-ram_size); +hival = cpu_to_be32(binfo-ram_size 32); +if (scells 1) { +mem_reg_property[acells + scells - 2] = hival; +} else if (hival != 0) { +fprintf(stderr, qemu: dtb file not compatible with +RAM size 4GB\n); +exit(1); +} + rc = qemu_devtree_setprop(fdt, /memory, reg, mem_reg_property, - sizeof(mem_reg_property)); + mem_reg_propsize * sizeof(uint32_t)); if (rc 0) { fprintf(stderr, couldn't set /memory/reg\n); } -- 1.7.5.4
Re: [Qemu-devel] [PATCH v2 1/2] fdc: fix relative seek
On 07/16/2012 03:24 PM, Kevin Wolf wrote: Am 16.07.2012 14:25, schrieb Pavel Hrdina: Signed-off-by: Pavel Hrdina phrd...@redhat.com --- hw/fdc.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) I applied both to the block branch for now. This restores the behaviour as it was before 6be01b1e. However, I believe it is still wrong: The direction should be interpreted the other way round, i.e. seek_out should decrease the cylinder number and seek_in should increase it. Do you have a guest that actually uses this command? Kevin I have host with real floppy device and I could send command directly, so I'll check this behavior. Pavel
Re: [Qemu-devel] [PATCH v2 1/2] fdc: fix relative seek
On 07/16/2012 03:26 PM, Pavel Hrdina wrote: On 07/16/2012 03:24 PM, Kevin Wolf wrote: Am 16.07.2012 14:25, schrieb Pavel Hrdina: Signed-off-by: Pavel Hrdina phrd...@redhat.com --- hw/fdc.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) I applied both to the block branch for now. This restores the behaviour as it was before 6be01b1e. However, I believe it is still wrong: The direction should be interpreted the other way round, i.e. seek_out should decrease the cylinder number and seek_in should increase it. Do you have a guest that actually uses this command? Kevin I have host with real floppy device and I could send command directly, so I'll check this behavior. Pavel Well, you're right... seek in increase track and seek out decrease track. I suspect that it was correct. I'll send v3 with this fix. Pavel
[Qemu-devel] [PATCH v2 0/6] arm_boot/vexpress-a15: Support 4GB of RAM
From: Peter Maydell petmay01@cam-vm-266.(none) This patchset adds support for booting with 4GB of RAM on the Versatile Express Cortex-A15 model. There are some caveats: * you need an LPAE A15 kernel * you need to be booting with device tree * your device tree blob needs to specify #address-cells and #size-cells as 2 (so addresses and sizes are 64 bit), which means you'll need to tweak the stock kernel dtb [the dtb files for the vexpress boards should be getting this change soon, although I'm not sure when it will hit upstream] * you need a minor kernel patch which stops the kernel throwing away the high 32 bits of the RAM size: http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7465/1 [hopefully coming soon to an upstream kernel near you] * you need a 64 bit host, obviously Changes v1-v2: * now based on master, no other dependencies * use uint64_t rather than target_phys_addr_t for sizes * removed debug tracing * tweaked ram size too big for ATAGS boot message NB that (following discussion with Peter C) the device_tree.c patch is unchanged from v1. Peter Maydell (6): hw/arm_boot.c: Make ram_size a uint64_t hw/arm_boot.c: Consistently use ram_size from arm_boot_info struct hw/arm_boot.c: Check for RAM sizes exceeding ATAGS capacity device_tree: Add support for reading device tree properties hw/arm_boot.c: Support DTBs which use 64 bit addresses hw/vexpress.c: Allow 4GB of RAM for Cortex-A15 daughterboard device_tree.c | 30 ++ device_tree.h |4 hw/arm-misc.h |2 +- hw/arm_boot.c | 46 +- hw/vexpress.c | 13 ++--- 5 files changed, 86 insertions(+), 9 deletions(-) -- 1.7.5.4
Re: [Qemu-devel] [PATCH 07/12] qemu-thread: add QemuSemaphore
On 2012-07-16 15:20, Paolo Bonzini wrote: Il 16/07/2012 14:00, Jan Kiszka ha scritto: On 2012-07-16 12:42, Paolo Bonzini wrote: The new thread pool will use semaphores instead of condition variables, because QemuCond does not have qemu_cond_timedwait. I'll post an updated patch (according to last round's review comments) that adds this service for POSIX. I bet you'll find a way to extend it to Win32 if that is required. ;) I can do that (or just use pthreads-win32), but only at the cost of making cond_wait() slower and more complex. Why will it affect cond_wait? WaitForSingleObject can time out as well. (I also like it more this way, since semaphores model well the producer-consumer problem). Let's not introduce another synchronization mechanism unless there is a real need. Semaphores tend to be misused for things they don't fit, so better keep them out of reach. That's what patch review is for... But even better is avoiding the temptation. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 07/12] qemu-thread: add QemuSemaphore
Il 16/07/2012 15:34, Jan Kiszka ha scritto: On 2012-07-16 15:20, Paolo Bonzini wrote: Il 16/07/2012 14:00, Jan Kiszka ha scritto: On 2012-07-16 12:42, Paolo Bonzini wrote: The new thread pool will use semaphores instead of condition variables, because QemuCond does not have qemu_cond_timedwait. I'll post an updated patch (according to last round's review comments) that adds this service for POSIX. I bet you'll find a way to extend it to Win32 if that is required. ;) I can do that (or just use pthreads-win32), but only at the cost of making cond_wait() slower and more complex. Why will it affect cond_wait? WaitForSingleObject can time out as well. qemu_cond_wait only uses WaitForSingleObject with INFINITE timeout, and the algorithm relies on that. Paolo
[Qemu-devel] [PATCH v2 2/6] hw/arm_boot.c: Consistently use ram_size from arm_boot_info struct
Clean up the mix of getting the RAM size from the global ram_size and from the ram_size field in the arm_boot_info structure, so that we always use the structure field. Signed-off-by: Peter Maydell peter.mayd...@linaro.org Reviewed-by: Peter A. G. Crosthwaite peter.crosthwa...@petalogix.com --- hw/arm_boot.c |5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/arm_boot.c b/hw/arm_boot.c index a1e6ddb..29ae324 100644 --- a/hw/arm_boot.c +++ b/hw/arm_boot.c @@ -357,7 +357,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) if (kernel_size 0) { entry = info-loader_start + KERNEL_LOAD_ADDR; kernel_size = load_image_targphys(info-kernel_filename, entry, - ram_size - KERNEL_LOAD_ADDR); + info-ram_size - KERNEL_LOAD_ADDR); is_linux = 1; } if (kernel_size 0) { @@ -371,7 +371,8 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) initrd_size = load_image_targphys(info-initrd_filename, info-loader_start + INITRD_LOAD_ADDR, - ram_size - INITRD_LOAD_ADDR); + info-ram_size + - INITRD_LOAD_ADDR); if (initrd_size 0) { fprintf(stderr, qemu: could not load initrd '%s'\n, info-initrd_filename); -- 1.7.5.4
[Qemu-devel] [PATCH v2 1/6] hw/arm_boot.c: Make ram_size a uint64_t
Make the RAM size in arm_boot_info a uint64_t so it can express the larger RAM sizes that may be seen in LPAE systems. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- hw/arm-misc.h |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/hw/arm-misc.h b/hw/arm-misc.h index 1f96229..bdd8fec 100644 --- a/hw/arm-misc.h +++ b/hw/arm-misc.h @@ -25,7 +25,7 @@ qemu_irq *armv7m_init(MemoryRegion *address_space_mem, /* arm_boot.c */ struct arm_boot_info { -int ram_size; +uint64_t ram_size; const char *kernel_filename; const char *kernel_cmdline; const char *initrd_filename; -- 1.7.5.4
[Qemu-devel] [PATCH v2 3/6] hw/arm_boot.c: Check for RAM sizes exceeding ATAGS capacity
The legacy ATAGS format for passing information to the kernel only allows RAM sizes which fit in 32 bits; enforce this restriction rather than silently doing something weird. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- hw/arm_boot.c |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/hw/arm_boot.c b/hw/arm_boot.c index 29ae324..af71ed6 100644 --- a/hw/arm_boot.c +++ b/hw/arm_boot.c @@ -399,6 +399,12 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) bootloader[5] = dtb_start; } else { bootloader[5] = info-loader_start + KERNEL_ARGS_ADDR; +if (info-ram_size = (1ULL 32)) { +fprintf(stderr, qemu: RAM size must be less than 4GB to boot + Linux kernel using ATAGS (try passing a device tree + using -dtb)\n); +exit(1); +} } bootloader[6] = entry; for (n = 0; n sizeof(bootloader) / 4; n++) { -- 1.7.5.4
[Qemu-devel] [PATCH v3 1/2] fdc: fix relative seek
Signed-off-by: Pavel Hrdina phrd...@redhat.com --- hw/fdc.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/hw/fdc.c b/hw/fdc.c index edf0706..9f84931 100644 --- a/hw/fdc.c +++ b/hw/fdc.c @@ -1695,7 +1695,7 @@ static void fdctrl_handle_drive_specification_command(FDCtrl *fdctrl, int direct } } -static void fdctrl_handle_relative_seek_out(FDCtrl *fdctrl, int direction) +static void fdctrl_handle_relative_seek_in(FDCtrl *fdctrl, int direction) { FDrive *cur_drv; @@ -1705,14 +1705,15 @@ static void fdctrl_handle_relative_seek_out(FDCtrl *fdctrl, int direction) fd_seek(cur_drv, cur_drv-head, cur_drv-max_track - 1, cur_drv-sect, 1); } else { -fd_seek(cur_drv, cur_drv-head, fdctrl-fifo[2], cur_drv-sect, 1); +fd_seek(cur_drv, cur_drv-head, +cur_drv-track + fdctrl-fifo[2], cur_drv-sect, 1); } fdctrl_reset_fifo(fdctrl); /* Raise Interrupt */ fdctrl_raise_irq(fdctrl, FD_SR0_SEEK); } -static void fdctrl_handle_relative_seek_in(FDCtrl *fdctrl, int direction) +static void fdctrl_handle_relative_seek_out(FDCtrl *fdctrl, int direction) { FDrive *cur_drv; @@ -1721,7 +1722,8 @@ static void fdctrl_handle_relative_seek_in(FDCtrl *fdctrl, int direction) if (fdctrl-fifo[2] cur_drv-track) { fd_seek(cur_drv, cur_drv-head, 0, cur_drv-sect, 1); } else { -fd_seek(cur_drv, cur_drv-head, fdctrl-fifo[2], cur_drv-sect, 1); +fd_seek(cur_drv, cur_drv-head, +cur_drv-track - fdctrl-fifo[2], cur_drv-sect, 1); } fdctrl_reset_fifo(fdctrl); /* Raise Interrupt */ -- 1.7.10.4
[Qemu-devel] [PATCH v3 2/2] fdc-test: introduce test_relative_seek
Signed-off-by: Pavel Hrdina phrd...@redhat.com --- tests/fdc-test.c | 46 +- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/tests/fdc-test.c b/tests/fdc-test.c index 585fb0e..10d11a4 100644 --- a/tests/fdc-test.c +++ b/tests/fdc-test.c @@ -47,9 +47,11 @@ enum { }; enum { -CMD_SENSE_INT = 0x08, -CMD_SEEK= 0x0f, -CMD_READ= 0xe6, +CMD_SENSE_INT = 0x08, +CMD_SEEK= 0x0f, +CMD_READ= 0xe6, +CMD_RELATIVE_SEEK_OUT = 0x8f, +CMD_RELATIVE_SEEK_IN= 0xcf, }; enum { @@ -91,13 +93,17 @@ static uint8_t floppy_recv(void) return inb(FLOPPY_BASE + reg_fifo); } -static void ack_irq(void) +static uint8_t ack_irq(void) { +uint8_t ret; + g_assert(get_irq(FLOPPY_IRQ)); floppy_send(CMD_SENSE_INT); floppy_recv(); -floppy_recv(); +ret = floppy_recv(); g_assert(!get_irq(FLOPPY_IRQ)); + +return ret; } static uint8_t send_read_command(void) @@ -281,6 +287,35 @@ static void test_sense_interrupt(void) floppy_recv(); } +static void test_relative_seek(void) +{ +uint8_t drive = 0; +uint8_t head = 0; +uint8_t cyl = 1; +uint8_t ret; + +/* Send seek to track 0 */ +send_step_pulse(0); + +/* Send relative seek to increase track by 1 */ +floppy_send(CMD_RELATIVE_SEEK_IN); +floppy_send(head 2 | drive); +g_assert(!get_irq(FLOPPY_IRQ)); +floppy_send(cyl); + +ret = ack_irq(); +g_assert(ret == 1); + +/* Send relative seek to decrease track by 1 */ +floppy_send(CMD_RELATIVE_SEEK_OUT); +floppy_send(head 2 | drive); +g_assert(!get_irq(FLOPPY_IRQ)); +floppy_send(cyl); + +ret = ack_irq(); +g_assert(ret == 0); +} + /* success if no crash or abort */ static void fuzz_registers(void) { @@ -329,6 +364,7 @@ int main(int argc, char **argv) qtest_add_func(/fdc/read_without_media, test_read_without_media); qtest_add_func(/fdc/media_change, test_media_change); qtest_add_func(/fdc/sense_interrupt, test_sense_interrupt); +qtest_add_func(/fdc/relative_seek, test_relative_seek); qtest_add_func(/fdc/fuzz-registers, fuzz_registers); ret = g_test_run(); -- 1.7.10.4
Re: [Qemu-devel] [PATCH 07/12] qemu-thread: add QemuSemaphore
On 2012-07-16 15:35, Paolo Bonzini wrote: Il 16/07/2012 15:34, Jan Kiszka ha scritto: On 2012-07-16 15:20, Paolo Bonzini wrote: Il 16/07/2012 14:00, Jan Kiszka ha scritto: On 2012-07-16 12:42, Paolo Bonzini wrote: The new thread pool will use semaphores instead of condition variables, because QemuCond does not have qemu_cond_timedwait. I'll post an updated patch (according to last round's review comments) that adds this service for POSIX. I bet you'll find a way to extend it to Win32 if that is required. ;) I can do that (or just use pthreads-win32), but only at the cost of making cond_wait() slower and more complex. Why will it affect cond_wait? WaitForSingleObject can time out as well. qemu_cond_wait only uses WaitForSingleObject with INFINITE timeout, and the algorithm relies on that. I see. But this doesn't look complex awfully. Just move the waker signaling from within cond_wait under the mutex as well, maybe add another flag if there is really someone waiting, and that's it. The costs should be hard to measure, even in line of code. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
[Qemu-devel] [PATCH] qemu-options.hx: Improve -{read|write}config options description
From: Michal Novotny minov...@redhat.com This is the patch to improve description for -{read|write}config functions. Signed-off-by: Michal Novotny minov...@redhat.com --- qemu-options.hx |8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index e15252e..97245a3 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2713,7 +2713,9 @@ DEF(readconfig, HAS_ARG, QEMU_OPTION_readconfig, STEXI @item -readconfig @var{file} @findex -readconfig -Read device configuration from @var{file}. +Read device configuration from @var{file}. This approach is useful when you want to spawn +QEMU process with many command line options but you don't want to exceed the command line +character limit. ETEXI DEF(writeconfig, HAS_ARG, QEMU_OPTION_writeconfig, -writeconfig file\n @@ -2721,7 +2723,9 @@ DEF(writeconfig, HAS_ARG, QEMU_OPTION_writeconfig, STEXI @item -writeconfig @var{file} @findex -writeconfig -Write device configuration to @var{file}. +Write device configuration to @var{file}. The @var{file} can be either filename to save +command line and device configuration into file or dash @code{-}) character to print the +output to stdout. This can be later used as input file for @code{-readconfig} option. ETEXI DEF(nodefconfig, 0, QEMU_OPTION_nodefconfig, -nodefconfig\n -- 1.7.10.4
[Qemu-devel] [PATCH] qemu-options.hx: Improve -nodefaults description
From: Michal Novotny minov...@redhat.com This patch improves the description of -nodefaults QEMU command line option by adding more information what is being disabled using this command. Signed-off-by: Michal Novotny minov...@redhat.com --- qemu-options.hx |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/qemu-options.hx b/qemu-options.hx index ecf7ca1..54ec14d 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2641,7 +2641,10 @@ DEF(nodefaults, 0, QEMU_OPTION_nodefaults, \ STEXI @item -nodefaults @findex -nodefaults -Don't create default devices. +Don't create default devices. Normally, QEMU sets the default devices like serial +port, parallel port, virtual console, monitor device, VGA adapter, floppy and +CD-ROM drive and others. The @code{-nodefaults} option will disable all those +default devices. ETEXI #ifndef _WIN32 -- 1.7.10.4
Re: [Qemu-devel] [PATCH v3 1/2] fdc: fix relative seek
Am 16.07.2012 15:48, schrieb Pavel Hrdina: Signed-off-by: Pavel Hrdina phrd...@redhat.com Thanks, applied both to the block branch. Kevin
[Qemu-devel] [PATCH v2 6/6] hw/vexpress.c: Allow 4GB of RAM for Cortex-A15 daughterboard
Now that we have LPAE support and can handle passing 64 bit RAM sizes to Linux via the device tree, we can lift the restriction in the Versatile Express A15 daughterboard model on not having more than 2GB of RAM. Allow up to 30GB, which is the maximum that can fit in the address map before running into the (unmodelled) aliases of the first 2GB. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- hw/vexpress.c | 13 ++--- 1 files changed, 10 insertions(+), 3 deletions(-) diff --git a/hw/vexpress.c b/hw/vexpress.c index 8072c5a..b2dc8a5 100644 --- a/hw/vexpress.c +++ b/hw/vexpress.c @@ -284,9 +284,16 @@ static void a15_daughterboard_init(const VEDBoardInfo *daughterboard, cpu_irq[n] = irqp[ARM_PIC_CPU_IRQ]; } -if (ram_size 0x8000) { -fprintf(stderr, vexpress-a15: cannot model more than 2GB RAM\n); -exit(1); +{ +/* We have to use a separate 64 bit variable here to avoid the gcc + * comparison is always false due to limited range of data type + * warning if we are on a host where ram_addr_t is 32 bits. + */ +uint64_t rsz = ram_size; +if (rsz (30ULL * 1024 * 1024 * 1024)) { +fprintf(stderr, vexpress-a15: cannot model more than 30GB RAM\n); +exit(1); +} } memory_region_init_ram(ram, vexpress.highmem, ram_size); -- 1.7.5.4
Re: [Qemu-devel] [PATCH 07/12] qemu-thread: add QemuSemaphore
Il 16/07/2012 15:53, Jan Kiszka ha scritto: qemu_cond_wait only uses WaitForSingleObject with INFINITE timeout, and the algorithm relies on that. I see. But this doesn't look complex awfully. Just move the waker signaling from within cond_wait under the mutex as well, maybe add another flag if there is really someone waiting, and that's it. The costs should be hard to measure, even in line of code. There is still a race after WaitForSingleObject times out. You need to catch the mutex before decreasing the number of waiters, and during that window somebody can broadcast the condition variable. I'm not saying it's impossible, just that it's hard and I dislike qemu_cond_timedwait as much as you dislike semaphores. :) Paolo
Re: [Qemu-devel] [PATCH 07/12] qemu-thread: add QemuSemaphore
On 2012-07-16 16:03, Paolo Bonzini wrote: Il 16/07/2012 15:53, Jan Kiszka ha scritto: qemu_cond_wait only uses WaitForSingleObject with INFINITE timeout, and the algorithm relies on that. I see. But this doesn't look complex awfully. Just move the waker signaling from within cond_wait under the mutex as well, maybe add another flag if there is really someone waiting, and that's it. The costs should be hard to measure, even in line of code. There is still a race after WaitForSingleObject times out. You need to catch the mutex before decreasing the number of waiters, and during that window somebody can broadcast the condition variable. ...and that's why you check what needs to be done to handle this race after grabbing the mutex. IOW, replicate the state information that the Windows semaphore contains into the emulated condition variable object. I'm not saying it's impossible, just that it's hard and I dislike qemu_cond_timedwait as much as you dislike semaphores. :) As you cannot eliminate our use cases for condition variables, let's focus on the existing synchronization patterns instead of introducing new ones + new mechanisms. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 07/12] qemu-thread: add QemuSemaphore
Il 16/07/2012 16:09, Jan Kiszka ha scritto: On 2012-07-16 16:03, Paolo Bonzini wrote: Il 16/07/2012 15:53, Jan Kiszka ha scritto: qemu_cond_wait only uses WaitForSingleObject with INFINITE timeout, and the algorithm relies on that. I see. But this doesn't look complex awfully. Just move the waker signaling from within cond_wait under the mutex as well, maybe add another flag if there is really someone waiting, and that's it. The costs should be hard to measure, even in line of code. There is still a race after WaitForSingleObject times out. You need to catch the mutex before decreasing the number of waiters, and during that window somebody can broadcast the condition variable. ...and that's why you check what needs to be done to handle this race after grabbing the mutex. IOW, replicate the state information that the Windows semaphore contains into the emulated condition variable object. It is already there (cv-waiters), but it is accessed atomically. To do what you suggest I would need to add a mutex. I'm not saying it's impossible, just that it's hard and I dislike qemu_cond_timedwait as much as you dislike semaphores. :) As you cannot eliminate our use cases for condition variables, let's focus on the existing synchronization patterns instead of introducing new ones + new mechanisms. I'm not introducing fancy new primitive of the day... semaphores have the advantage of having better support under Windows (at least older versions; Microsoft saw the light starting with Vista---yes I know the oxymoron---and added condvars). Paolo
Re: [Qemu-devel] [PATCH] RFC: vfio-powerpc: added VFIO support (v3)
On Sat, 2012-07-14 at 12:34 +1000, Alexey Kardashevskiy wrote: On 14/07/12 01:07, Alex Williamson wrote: On Fri, 2012-07-13 at 17:26 +1000, Alexey Kardashevskiy wrote: It literally does the following: 1. POWERPC IOMMU support (the kernel counterpart is required) 2. The patch assumes that IOAPIC calls are going to be replaced with something generic. 3. vfio_group_iommu_ioctl() has been added to let sPAPR IOMMU handler to call VFIO IOMMU driver. 4. Change sPAPR PHB to scan the PCI bus which is used for the IOMMU-VFIO group. Now it is enough to add the following to the QEMU command line to get VFIO up with all the devices from IOMMU group with id=3: -device spapr-pci-host-bridge,busname=E1000E,buid=0x3,iommu=3,\ mem_win_addr=0x2300,io_win_addr=0x2400,msi_win_addr=0x2500 Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru --- hw/ppc/Makefile.objs |3 ++ hw/spapr.h|4 ++ hw/spapr_iommu.c | 69 ++- hw/spapr_iommu_vfio.h | 49 ++ hw/spapr_pci.c| 108 ++--- hw/spapr_pci.h|4 ++ hw/vfio_pci.c | 30 ++ hw/vfio_pci.h |2 + trace-events |1 + 9 files changed, 264 insertions(+), 6 deletions(-) create mode 100644 hw/spapr_iommu_vfio.h diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs index f573a95..c46a049 100644 --- a/hw/ppc/Makefile.objs +++ b/hw/ppc/Makefile.objs @@ -25,4 +25,7 @@ obj-$(CONFIG_FDT) += ../device_tree.o # Xilinx PPC peripherals obj-y += xilinx_ethlite.o +# VFIO PCI device assignment +obj-$(CONFIG_VFIO_PCI) += vfio_pci.o + obj-y := $(addprefix ../,$(obj-y)) diff --git a/hw/spapr.h b/hw/spapr.h index b37f337..26e26f6 100644 --- a/hw/spapr.h +++ b/hw/spapr.h @@ -340,4 +340,8 @@ int spapr_dma_dt(void *fdt, int node_off, const char *propname, int spapr_tcet_dma_dt(void *fdt, int node_off, const char *propname, DMAContext *dma); +void spapr_vfio_init_dma(int group_id, uint32_t liobn, + uint64_t *dma32_window_start, + uint64_t *dma32_window_size); + #endif /* !defined (__HW_SPAPR_H__) */ diff --git a/hw/spapr_iommu.c b/hw/spapr_iommu.c index 50c288d..e48ced1 100644 --- a/hw/spapr_iommu.c +++ b/hw/spapr_iommu.c @@ -23,6 +23,8 @@ #include dma.h #include hw/spapr.h +#include hw/spapr_iommu_vfio.h +#include hw/vfio_pci.h #include libfdt.h @@ -183,6 +185,67 @@ static int put_tce_emu(target_ulong liobn, target_ulong ioba, target_ulong tce) return 0; } +typedef struct sPAPRVFIOTable { +int group_id; +uint32_t liobn; +QLIST_ENTRY(sPAPRVFIOTable) list; +} sPAPRVFIOTable; + +QLIST_HEAD(vfio_tce_tables, sPAPRVFIOTable) vfio_tce_tables; + +void spapr_vfio_init_dma(int group_id, uint32_t liobn, + uint64_t *dma32_window_start, + uint64_t *dma32_window_size) +{ +sPAPRVFIOTable *t; +struct tce_iommu_info info = { .argsz = sizeof(info) }; + +if (vfio_group_iommu_ioctl(group_id, SPAPR_TCE_IOMMU_GET_INFO, info)) { +perror(SPAPR_TCE_IOMMU_GET_INFO failed); +return; +} +*dma32_window_start = info.dma32_window_start; +*dma32_window_size = info.dma32_window_size; + +t = g_malloc0(sizeof(*t)); +t-group_id = group_id; +t-liobn = liobn; + +QLIST_INSERT_HEAD(vfio_tce_tables, t, list); +} + +static int put_tce_vfio(uint32_t liobn, target_ulong ioba, target_ulong tce) +{ +sPAPRVFIOTable *t; +struct tce_iommu_dma_map map = { +.argsz = sizeof(map), +.va = 0, +.dmaaddr = ioba, +}; + +QLIST_FOREACH(t, vfio_tce_tables, list) { +if (t-liobn != liobn) { +continue; +} +if (tce) { +map.va = (uintptr_t)qemu_get_ram_ptr(tce ~SPAPR_TCE_PAGE_MASK); +if (vfio_group_iommu_ioctl(t-group_id, SPAPR_TCE_IOMMU_MAP_DMA, + map)) { +perror(TCE_MAP_DMA); +return H_PARAMETER; +} +} else { +if (vfio_group_iommu_ioctl(t-group_id, SPAPR_TCE_IOMMU_UNMAP_DMA, + map)) { +perror(TCE_UNMAP_DMA); +return H_PARAMETER; +} +} +return H_SUCCESS; +} +return H_CONTINUE; /* positive non-zero value */ +} + static target_ulong h_put_tce(CPUPPCState *env, sPAPREnvironment *spapr, target_ulong opcode, target_ulong *args) { @@ -200,7 +263,11 @@ static target_ulong h_put_tce(CPUPPCState *env, sPAPREnvironment *spapr,
[Qemu-devel] [PATCH 2/2] virtio-scsi spec: add configuration change event
This adds an event for changes to LUN parameters, for example capacity. These are reported in virtio-blk via configuration changes, and we want a similar functionality in virtio-scsi too. There is no list of supported parameter changes, instead we just refer to the list of sense codes in the SCSI specification. This event will usually be serviced in one of three ways: 1) call an OS service to revalidate the disk, either always or only for some specific sense codes; 2) somehow pass the sense directly to the upper-level driver; 3) inject a TEST UNIT READY command into the upper-level device, so that the OS will see the unit attention code and react. Of course a mix of the three is also possible, depending on how the driver writer prefers to have his layering violations served. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- virtio-spec.lyx | 95 +++ 1 file changed, 95 insertions(+) diff --git a/virtio-spec.lyx b/virtio-spec.lyx index f8b214b..8d2ac9a 100644 --- a/virtio-spec.lyx +++ b/virtio-spec.lyx @@ -6995,6 +6995,21 @@ VIRTIO_SCSI_F_HOTPLUG (1) The host should enable hot-plug/hot-unplug of new LUNs and targets on the SCSI bus. +\change_inserted 1531152142 1342440342 + +\end_layout + +\begin_layout Description + +\change_inserted 1531152142 1342440768 +VIRTIO_SCSI_F_CHANGE +\begin_inset space ~ +\end_inset + +(2) The host will report changes to LUN parameters via a VIRTIO_SCSI_T_PARAM_CHA +NGE event. +\change_unchanged + \end_layout \end_deeper @@ -8673,6 +8688,86 @@ reason \begin_layout Standard When dropped events are reported, the driver should poll for asynchronous events manually using SCSI commands. +\change_inserted 1531152142 1342439104 + +\end_layout + +\end_deeper +\begin_layout Description + +\change_inserted 1531152142 1342440778 +LUN +\begin_inset space ~ +\end_inset + +parameter +\begin_inset space ~ +\end_inset + +change +\begin_inset space ~ +\end_inset + + +\begin_inset Newline newline +\end_inset + + +\begin_inset listings +inline false +status open + +\begin_layout Plain Layout + +\change_inserted 1531152142 1342440783 + +#define VIRTIO_SCSI_T_PARAM_CHANGE 3 +\end_layout + +\end_inset + + +\end_layout + +\begin_deeper +\begin_layout Standard + +\change_inserted 1531152142 1342440882 +By sending this event, the device signals that the configuration parameters + (for example the capacity) of a logical unit have changed. + The +\series bold +event +\series default + field is set to VIRTIO_SCSI_T_PARAM_CHANGE. + The +\series bold +lun +\series default + field addresses a logical unit in the SCSI host. +\end_layout + +\begin_layout Standard + +\change_inserted 1531152142 1342440916 +The same event is also reported as a unit attention condition. + The +\series bold +reason +\series default + field contains the additional sense code and additional sense code qualifier, + respectively in bits 0..7 and 8..15. + For example, a change in capacity will be reported as asc 0x2a, ascq 0x09 + (CAPACITY DATA HAS CHANGED). +\end_layout + +\begin_layout Standard + +\change_inserted 1531152142 1342442803 +For MMC devices (inquiry type 5) there would be some overlap between this + event and the asynchronous notification event. + For simplicity, as of this version of the specification the host must + never report this event for MMC devices. \end_layout \end_deeper -- 1.7.10.4
[Qemu-devel] [PATCH 1/2] virtio-scsi spec: unify event structs
All currently defined event structs have the same fields. Simplify the driver by enforcing this also for future structs. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- virtio-spec.lyx | 69 +++ 1 file changed, 65 insertions(+), 4 deletions(-) diff --git a/virtio-spec.lyx b/virtio-spec.lyx index 905e619..f8b214b 100644 --- a/virtio-spec.lyx +++ b/virtio-spec.lyx @@ -8207,7 +8207,20 @@ struct virtio_scsi_event { \begin_layout Plain Layout +\change_deleted 1531152142 1342440791 + ... +\change_inserted 1531152142 1342440791 +u8 lun[8]; +\end_layout + +\begin_layout Plain Layout + +\change_inserted 1531152142 1342440791 + +u32 reason; +\change_unchanged + \end_layout \begin_layout Plain Layout @@ -8221,16 +8234,32 @@ struct virtio_scsi_event { \end_layout \begin_layout Standard -If bit 31 is set in the event field, the device failed to report an event - due to missing buffers. +If bit 31 is set in the +\series bold +event +\series default + field, the device failed to report an event due to missing buffers. In this case, the driver should poll the logical units for unit attention conditions, and/or do whatever form of bus scan is appropriate for the guest operating system. \end_layout \begin_layout Standard -Other data that the device writes to the buffer depends on the contents - of the event field. + +\change_deleted 1531152142 1342440830 +Other data that the device writes to the buffer +\change_inserted 1531152142 1342440839 +The meaning of the +\series bold +reason +\series default + field +\change_unchanged + depends on the contents of the +\series bold +event +\series default + field. The following events are defined: \end_layout @@ -8312,36 +8341,50 @@ status open \begin_layout Plain Layout +\change_deleted 1531152142 1342440799 + struct virtio_scsi_event_reset { \end_layout \begin_layout Plain Layout +\change_deleted 1531152142 1342440799 + // Write-only part \end_layout \begin_layout Plain Layout +\change_deleted 1531152142 1342440799 + u32 event; \end_layout \begin_layout Plain Layout +\change_deleted 1531152142 1342440799 + u8 lun[8]; \end_layout \begin_layout Plain Layout +\change_deleted 1531152142 1342440799 + u32 reason; \end_layout \begin_layout Plain Layout +\change_deleted 1531152142 1342440799 + } \end_layout \begin_layout Plain Layout +\change_deleted 1531152142 1342440799 + \end_layout \begin_layout Plain Layout @@ -8542,40 +8585,58 @@ status open \begin_layout Plain Layout #define VIRTIO_SCSI_T_ASYNC_NOTIFY 2 +\change_deleted 1531152142 1342440854 + \end_layout \begin_layout Plain Layout +\change_deleted 1531152142 1342440854 + \end_layout \begin_layout Plain Layout +\change_deleted 1531152142 1342440854 + struct virtio_scsi_event_an { \end_layout \begin_layout Plain Layout +\change_deleted 1531152142 1342440854 + // Write-only part \end_layout \begin_layout Plain Layout +\change_deleted 1531152142 1342440854 + u32 event; \end_layout \begin_layout Plain Layout +\change_deleted 1531152142 1342440854 + u8 lun[8]; \end_layout \begin_layout Plain Layout +\change_deleted 1531152142 1342440854 + u32 reason; \end_layout \begin_layout Plain Layout +\change_deleted 1531152142 1342440854 + } +\change_unchanged + \end_layout \end_inset -- 1.7.10.4
[Qemu-devel] [PATCH 0/2] virtio-scsi spec: event improvements
This makes some changes to the virtio-scsi event specification, so that it is now possible to use virtio-scsi events in the implementation of the QEMU block_resize command. Thanks to Cong Meng for finally implementing virtio-scsi hotplug, which made me look at block_resize again! Paolo Bonzini (2): virtio-scsi spec: unify event structs virtio-scsi spec: add configuration change event virtio-spec.lyx | 164 +-- 1 file changed, 160 insertions(+), 4 deletions(-) -- 1.7.10.4
[Qemu-devel] [PATCH 0/5] virtio-scsi: support block_resize
This series adds support for block_resize to virtio-scsi. Events are reported via a new event type. Patches to the spec are on the list. Paolo Bonzini (5): scsi-disk: removable hard disks support START/STOP scsi-disk: report resized disk via sense codes scsi: establish precedence levels for unit attention scsi: report parameter changes to HBA drivers virtio-scsi: report parameter change events hw/scsi-bus.c| 67 +- hw/scsi-disk.c | 21 +++-- hw/scsi.h|5 hw/virtio-scsi.c | 16 + trace-events |1 + 5 files changed, 102 insertions(+), 8 deletions(-) -- 1.7.10.4
Re: [Qemu-devel] [PATCH v2 1/3] linux-user: pass sockaddr from host to target
On 16 July 2012 11:13, Jing Huang jing.huang@gmail.com wrote: This patch pass sockaddr from host to target. Signed-off-by: Jing Huang jing.huang@gmail.com --- linux-user/syscall.c | 10 +- 1 files changed, 9 insertions(+), 1 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 539af3f..28c8ba5 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -1873,8 +1873,16 @@ static abi_long do_sendrecvmsg(int fd, abi_ulong target_msg, if (!is_error(ret)) { len = ret; ret = host_to_target_cmsg(msgp, msg); -if (!is_error(ret)) +if (!is_error(ret)) { +msgp-msg_namelen = msg.msg_namelen; You need to swap msg_namelen using tswap32(). +ret = host_to_target_sockaddr(tswapal(msgp-msg_name), +msg.msg_name, msg.msg_namelen); host_to_target_sockaddr assumes msg.msg_name isn't NULL, so you need to enclose this in an if() to avoid that. +if (ret) { +qemu_log(Failed to pass sockaddr to target guest); Don't print any logging here -- we should only be logging anything for (a) unimplemented features or (b) debug tracing which is guarded by #ifdef DEBUG. +return ret; Returning here bypasses the unlocking of the iovec and user struct. +} ret = len; ...the above two points mean you wanted if (!is_error(ret)) { ret = len; } here. +} -- PMM
[Qemu-devel] [PATCH 0/2] More RELATIVE SEEK fun
Hi Pavel, you said you have a real floppy drive around. Would you mind giving the cases from patch 2 a try on it? The spec wasn't entirely clear to me, so the values in the test case depend more on guessing than on knowledge. I'm pretty sure that qemu is buggy there, though, so if you like to fix first the test according to real hardware and then qemu to pass the test, that would be great. Kevin Kevin Wolf (2): fdc-test: Clean up a bit fdc-test: Check RELATIVE SEEK beyond track 0/80 tests/fdc-test.c | 72 ++--- 1 files changed, 51 insertions(+), 21 deletions(-) -- 1.7.6.5
[Qemu-devel] [PATCH 1/2] fdc-test: Clean up a bit
Readability of the test code has suffered as the test case evolved. This should improve it a bit again. Signed-off-by: Kevin Wolf kw...@redhat.com --- tests/fdc-test.c | 36 1 files changed, 20 insertions(+), 16 deletions(-) diff --git a/tests/fdc-test.c b/tests/fdc-test.c index 10d11a4..fa74411 100644 --- a/tests/fdc-test.c +++ b/tests/fdc-test.c @@ -93,17 +93,21 @@ static uint8_t floppy_recv(void) return inb(FLOPPY_BASE + reg_fifo); } -static uint8_t ack_irq(void) +/* pcn: Present Cylinder Number */ +static void ack_irq(uint8_t *pcn) { uint8_t ret; g_assert(get_irq(FLOPPY_IRQ)); floppy_send(CMD_SENSE_INT); floppy_recv(); + ret = floppy_recv(); -g_assert(!get_irq(FLOPPY_IRQ)); +if (pcn != NULL) { +*pcn = ret; +} -return ret; +g_assert(!get_irq(FLOPPY_IRQ)); } static uint8_t send_read_command(void) @@ -162,7 +166,7 @@ static uint8_t send_read_command(void) return ret; } -static void send_step_pulse(int cyl) +static void send_seek(int cyl) { int drive = 0; int head = 0; @@ -171,7 +175,7 @@ static void send_step_pulse(int cyl) floppy_send(head 2 | drive); g_assert(!get_irq(FLOPPY_IRQ)); floppy_send(cyl); -ack_irq(); +ack_irq(NULL); } static uint8_t cmos_read(uint8_t reg) @@ -198,7 +202,7 @@ static void test_no_media_on_start(void) assert_bit_set(dir, DSKCHG); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); -send_step_pulse(1); +send_seek(1); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); dir = inb(FLOPPY_BASE + reg_dir); @@ -229,14 +233,14 @@ static void test_media_change(void) dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); -send_step_pulse(0); +send_seek(0); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); /* Step to next track should clear DSKCHG bit. */ -send_step_pulse(1); +send_seek(1); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_clear(dir, DSKCHG); dir = inb(FLOPPY_BASE + reg_dir); @@ -252,13 +256,13 @@ static void test_media_change(void) dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); -send_step_pulse(0); +send_seek(0); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); -send_step_pulse(1); +send_seek(1); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); dir = inb(FLOPPY_BASE + reg_dir); @@ -292,10 +296,10 @@ static void test_relative_seek(void) uint8_t drive = 0; uint8_t head = 0; uint8_t cyl = 1; -uint8_t ret; +uint8_t pcn; /* Send seek to track 0 */ -send_step_pulse(0); +send_seek(0); /* Send relative seek to increase track by 1 */ floppy_send(CMD_RELATIVE_SEEK_IN); @@ -303,8 +307,8 @@ static void test_relative_seek(void) g_assert(!get_irq(FLOPPY_IRQ)); floppy_send(cyl); -ret = ack_irq(); -g_assert(ret == 1); +ack_irq(pcn); +g_assert(pcn == 1); /* Send relative seek to decrease track by 1 */ floppy_send(CMD_RELATIVE_SEEK_OUT); @@ -312,8 +316,8 @@ static void test_relative_seek(void) g_assert(!get_irq(FLOPPY_IRQ)); floppy_send(cyl); -ret = ack_irq(); -g_assert(ret == 0); +ack_irq(pcn); +g_assert(pcn == 0); } /* success if no crash or abort */ -- 1.7.6.5
[Qemu-devel] [PATCH 2/2] fdc-test: Check RELATIVE SEEK beyond track 0/80
TODO This needs to be checked against a real drive Signed-off-by: Kevin Wolf kw...@redhat.com --- tests/fdc-test.c | 48 +--- 1 files changed, 37 insertions(+), 11 deletions(-) diff --git a/tests/fdc-test.c b/tests/fdc-test.c index fa74411..56e745a 100644 --- a/tests/fdc-test.c +++ b/tests/fdc-test.c @@ -94,13 +94,17 @@ static uint8_t floppy_recv(void) } /* pcn: Present Cylinder Number */ -static void ack_irq(uint8_t *pcn) +static void ack_irq(uint8_t *st0, uint8_t *pcn) { uint8_t ret; g_assert(get_irq(FLOPPY_IRQ)); floppy_send(CMD_SENSE_INT); -floppy_recv(); + +ret = floppy_recv(); +if (st0 != NULL) { +*st0 = ret; +} ret = floppy_recv(); if (pcn != NULL) { @@ -175,7 +179,7 @@ static void send_seek(int cyl) floppy_send(head 2 | drive); g_assert(!get_irq(FLOPPY_IRQ)); floppy_send(cyl); -ack_irq(NULL); +ack_irq(NULL, NULL); } static uint8_t cmos_read(uint8_t reg) @@ -295,29 +299,51 @@ static void test_relative_seek(void) { uint8_t drive = 0; uint8_t head = 0; -uint8_t cyl = 1; uint8_t pcn; +uint8_t st0; /* Send seek to track 0 */ send_seek(0); -/* Send relative seek to increase track by 1 */ +/* Send relative seek to increase track by 3 */ floppy_send(CMD_RELATIVE_SEEK_IN); floppy_send(head 2 | drive); g_assert(!get_irq(FLOPPY_IRQ)); -floppy_send(cyl); +floppy_send(3); -ack_irq(pcn); -g_assert(pcn == 1); +ack_irq(st0, pcn); +g_assert_cmpint(pcn, ==, 3); +g_assert_cmpint(st0, ==, 0x20); /* Send relative seek to decrease track by 1 */ floppy_send(CMD_RELATIVE_SEEK_OUT); floppy_send(head 2 | drive); g_assert(!get_irq(FLOPPY_IRQ)); -floppy_send(cyl); +floppy_send(1); + +ack_irq(st0, pcn); +g_assert_cmpint(pcn, ==, 2); +g_assert_cmpint(st0, ==, 0x20); + +/* Send relative seek to beyond track 0 */ +floppy_send(CMD_RELATIVE_SEEK_OUT); +floppy_send(head 2 | drive); +g_assert(!get_irq(FLOPPY_IRQ)); +floppy_send(42); + +ack_irq(st0, pcn); +g_assert_cmpint(pcn, ==, 0); +g_assert_cmpint(st0, ==, 0x70); + +/* Send try relative seek to beyond track 80 */ +floppy_send(CMD_RELATIVE_SEEK_IN); +floppy_send(head 2 | drive); +g_assert(!get_irq(FLOPPY_IRQ)); +floppy_send(200); -ack_irq(pcn); -g_assert(pcn == 0); +ack_irq(st0, pcn); +g_assert_cmpint(pcn, ==, 79); +g_assert_cmpint(st0, ==, 0x50); } /* success if no crash or abort */ -- 1.7.6.5
Re: [Qemu-devel] [PATCH v2 2/3] linux-user: make do_setsockopt support SOL_RAW ICMP_FILTER socket option
On 16 July 2012 11:14, Jing Huang jing.huang@gmail.com wrote: This patch makes do_setsockopt() support SOL_RAW ICMP_FILTER socket option. Signed-off-by: Jing Huang jing.huang@gmail.com --- linux-user/syscall.c | 20 1 files changed, 20 insertions(+), 0 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 28c8ba5..fc8690d 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -60,6 +60,7 @@ int __clone2(int (*fn)(void *), void *child_stack_base, #include netinet/ip.h #include netinet/tcp.h #include linux/wireless.h +#include linux/icmp.h #include qemu-common.h #ifdef TARGET_GPROF #include sys/gmon.h @@ -1442,6 +1443,25 @@ static abi_long do_setsockopt(int sockfd, int level, int optname, goto unimplemented; } break; +case SOL_RAW: +switch (optname) { +case ICMP_FILTER: +/*struct icmp_filter takes an u32 value*/ Please add spaces after the /* and before the */. +optname = ICMP_FILTER; Pointless assignment, as Dunrong says. +if (optlen sizeof(uint32_t)) { +return -TARGET_EINVAL; +} + +if (get_user_u32(val, optval_addr)) { +return -TARGET_EFAULT; +} +ret = get_errno(setsockopt(sockfd, level, optname, +(char *)val, sizeof(val))); Agreed with Dunrong, this is a pointless cast; setsockopt() takes a void* for the optval pointer. (Compare posix spec and see the other calls to setsockopt in this function. The prototype of the internal kernel function implementing setsockopt is not relevant because that is not the API that the kernel exposes to userspace.) +break; +default: +goto unimplemented; +} +break; case TARGET_SOL_SOCKET: switch (optname) { /* Options with 'int' argument. */ -- 1.7.8.6 -- PMM
[Qemu-devel] [PATCH 4/5] scsi: report parameter changes to HBA drivers
Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- hw/scsi-bus.c | 10 ++ hw/scsi-disk.c |2 +- hw/scsi.h |2 ++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c index d5e1fb0..77aa946 100644 --- a/hw/scsi-bus.c +++ b/hw/scsi-bus.c @@ -1072,6 +1072,16 @@ int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf) return 0; } +void scsi_device_report_change(SCSIDevice *dev, SCSISense sense) +{ +SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, dev-qdev.parent_bus); + +scsi_device_set_ua(dev, sense); +if (bus-info-change) { +bus-info-change(bus, dev, sense); +} +} + /* * Predefined sense codes */ diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index fabd0bf..8351de5 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -1867,7 +1867,7 @@ static void scsi_disk_resize_cb(void *opaque) { SCSIDiskState *s = opaque; -scsi_device_set_ua(s-qdev, SENSE_CODE(CAPACITY_CHANGED)); +scsi_device_report_change(s-qdev, SENSE_CODE(CAPACITY_CHANGED)); } static void scsi_cd_change_media_cb(void *opaque, bool load) diff --git a/hw/scsi.h b/hw/scsi.h index 4804be9..380bb89 100644 --- a/hw/scsi.h +++ b/hw/scsi.h @@ -132,6 +132,7 @@ struct SCSIBusInfo { void (*cancel)(SCSIRequest *req); void (*hotplug)(SCSIBus *bus, SCSIDevice *dev); void (*hot_unplug)(SCSIBus *bus, SCSIDevice *dev); +void (*change)(SCSIBus *bus, SCSIDevice *dev, SCSISense sense); QEMUSGList *(*get_sg_list)(SCSIRequest *req); void (*save_request)(QEMUFile *f, SCSIRequest *req); @@ -240,6 +241,7 @@ void scsi_req_cancel(SCSIRequest *req); void scsi_req_retry(SCSIRequest *req); void scsi_device_purge_requests(SCSIDevice *sdev, SCSISense sense); void scsi_device_set_ua(SCSIDevice *sdev, SCSISense sense); +void scsi_device_report_change(SCSIDevice *dev, SCSISense sense); int scsi_device_get_sense(SCSIDevice *dev, uint8_t *buf, int len, bool fixed); SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int target, int lun); -- 1.7.10.4
[Qemu-devel] [PATCH 3/5] scsi: establish precedence levels for unit attention
When a device is resized, we will report a unit attention condition for CAPACITY DATA HAS CHANGED. However, we should ensure that this condition does not override a more important unit attention condition. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- hw/scsi-bus.c | 52 +++- hw/scsi-disk.c |4 ++-- hw/scsi.h |1 + trace-events |1 + 4 files changed, 55 insertions(+), 3 deletions(-) diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c index 2547c50..d5e1fb0 100644 --- a/hw/scsi-bus.c +++ b/hw/scsi-bus.c @@ -1530,6 +1530,55 @@ void scsi_req_abort(SCSIRequest *req, int status) scsi_req_unref(req); } +static int scsi_ua_precedence(SCSISense sense) +{ +if (sense.key != UNIT_ATTENTION) { +return INT_MAX; +} +if (sense.asc == 0x29 sense.ascq == 0x04) { +/* DEVICE INTERNAL RESET goes with POWER ON OCCURRED */ +return 1; +} else if (sense.asc == 0x3F sense.ascq == 0x01) { +/* MICROCODE HAS BEEN CHANGED goes with SCSI BUS RESET OCCURRED */ +return 2; +} else if (sense.asc == 0x29 (sense.ascq == 0x05 || sense.ascq == 0x06)) { +/* These two go with all others. */ +; +} else if (sense.asc == 0x29 sense.ascq = 0x07) { +/* POWER ON, RESET OR BUS DEVICE RESET OCCURRED = 0 + * POWER ON OCCURRED = 1 + * SCSI BUS RESET OCCURRED = 2 + * BUS DEVICE RESET FUNCTION OCCURRED = 3 + * I_T NEXUS LOSS OCCURRED = 7 + */ +return sense.ascq; +} else if (sense.asc == 0x2F sense.ascq == 0x01) { +/* COMMANDS CLEARED BY POWER LOSS NOTIFICATION */ +return 8; +} +return (sense.asc 8) | sense.ascq; +} + +void scsi_device_set_ua(SCSIDevice *sdev, SCSISense sense) +{ +int prec1, prec2; +if (sense.key != UNIT_ATTENTION) { +return; +} +trace_scsi_device_set_ua(sdev-id, sdev-lun, sense.key, + sense.asc, sense.ascq); + +/* + * Override a pre-existing unit attention condition, except for a more + * important reset condition. +*/ +prec1 = scsi_ua_precedence(sdev-unit_attention); +prec2 = scsi_ua_precedence(sense); +if (prec2 prec1) { +sdev-unit_attention = sense; +} +} + void scsi_device_purge_requests(SCSIDevice *sdev, SCSISense sense) { SCSIRequest *req; @@ -1538,7 +1587,8 @@ void scsi_device_purge_requests(SCSIDevice *sdev, SCSISense sense) req = QTAILQ_FIRST(sdev-requests); scsi_req_cancel(req); } -sdev-unit_attention = sense; + +scsi_device_set_ua(sdev, sense); } static char *scsibus_get_dev_path(DeviceState *dev) diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 0905446..fabd0bf 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -1886,7 +1886,7 @@ static void scsi_cd_change_media_cb(void *opaque, bool load) */ s-media_changed = load; s-tray_open = !load; -s-qdev.unit_attention = SENSE_CODE(UNIT_ATTENTION_NO_MEDIUM); +scsi_device_set_ua(s-qdev, SENSE_CODE(UNIT_ATTENTION_NO_MEDIUM)); s-media_event = true; s-eject_request = false; } @@ -1925,7 +1925,7 @@ static void scsi_disk_unit_attention_reported(SCSIDevice *dev) SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev); if (s-media_changed) { s-media_changed = false; -s-qdev.unit_attention = SENSE_CODE(MEDIUM_CHANGED); +scsi_device_set_ua(s-qdev, SENSE_CODE(MEDIUM_CHANGED)); } } diff --git a/hw/scsi.h b/hw/scsi.h index e901350..4804be9 100644 --- a/hw/scsi.h +++ b/hw/scsi.h @@ -239,6 +239,7 @@ void scsi_req_abort(SCSIRequest *req, int status); void scsi_req_cancel(SCSIRequest *req); void scsi_req_retry(SCSIRequest *req); void scsi_device_purge_requests(SCSIDevice *sdev, SCSISense sense); +void scsi_device_set_ua(SCSIDevice *sdev, SCSISense sense); int scsi_device_get_sense(SCSIDevice *dev, uint8_t *buf, int len, bool fixed); SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int target, int lun); diff --git a/trace-events b/trace-events index 7baa42d..90e9c2a 100644 --- a/trace-events +++ b/trace-events @@ -393,6 +393,7 @@ scsi_req_parsed(int target, int lun, int tag, int cmd, int mode, int xfer) targ scsi_req_parsed_lba(int target, int lun, int tag, int cmd, uint64_t lba) target %d lun %d tag %d command %d lba %PRIu64 scsi_req_parse_bad(int target, int lun, int tag, int cmd) target %d lun %d tag %d command %d scsi_req_build_sense(int target, int lun, int tag, int key, int asc, int ascq) target %d lun %d tag %d key %#02x asc %#02x ascq %#02x +scsi_device_set_ua(int target, int lun, int key, int asc, int ascq) target %d lun %d key %#02x asc %#02x ascq %#02x scsi_report_luns(int target, int lun, int tag) target %d lun %d tag %d scsi_inquiry(int target, int lun, int tag, int cdb1, int cdb2) target %d lun %d tag %d page %#02x/%#02x scsi_test_unit_ready(int target, int lun, int tag) target %d lun %d tag %d --
Re: [Qemu-devel] [PATCH v3 1/2] bitops: drop volatile qualifier
On 07/14/2012 06:34 AM, Blue Swirl wrote: Qualifier 'volatile' is not useful for applications, it's too strict for single threaded code but does not give the real atomicity guarantees needed for multithreaded code. Drop them and now useless casts. -static inline void set_bit(int nr, volatile unsigned long *addr) +static inline void set_bit(int nr, unsigned long *addr) { unsigned long mask = BIT_MASK(nr); - unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr); +unsigned long *p = addr + BIT_WORD(nr); The diff looks weird, because you are converting TAB to space on your affected lines but not cleaning up the neighboring lines. Is it worth a preliminary patch to whitespace-sanitize things? -- 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 v2 3/3] linux-user: exclude SO_TIMESTAMP cmsg_type from unsuppoted ancillary data
On 16 July 2012 11:15, Jing Huang jing.huang@gmail.com wrote: This patch excludes SO_TIMESTAMP cmsg_type from unsuppoted ancillary data. unsupported. Signed-off-by: Jing Huang jing.huang@gmail.com --- linux-user/syscall.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index fc8690d..9fce57c 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -1349,7 +1349,9 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh, target_cmsg-cmsg_type = tswap32(cmsg-cmsg_type); target_cmsg-cmsg_len = tswapal(TARGET_CMSG_LEN(len)); -if (cmsg-cmsg_level != TARGET_SOL_SOCKET || cmsg-cmsg_type != SCM_RIGHTS) { +if (cmsg-cmsg_level != TARGET_SOL_SOCKET || +(cmsg-cmsg_type != SCM_RIGHTS +cmsg-cmsg_type != SO_TIMESTAMP)) { gemu_log(Unsupported ancillary data: %d/%d\n, cmsg-cmsg_level, cmsg-cmsg_type); memcpy(target_data, data, len); } else { This is kind of ugly -- it causes us to process a SO_TIMESTAMP payload (which is a 'struct timeval') in the same way as an SCM_RIGHTS payload (which is an array of file descriptors). This happens to work because a struct timeval is a pair of 32 bit integers, and so tswap32() all the words in the data works, but it looks pretty weird. If you want to do it this way you need at least an explanatory comment and really the variables 'fd', 'target_fd', 'numfds' would need to change to something more generic and less fd-specific. -- PMM
[Qemu-devel] [PATCH 5/5] virtio-scsi: report parameter change events
Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- hw/virtio-scsi.c | 16 1 file changed, 16 insertions(+) diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c index 83dbabd..80a47d7 100644 --- a/hw/virtio-scsi.c +++ b/hw/virtio-scsi.c @@ -27,6 +27,7 @@ /* Feature Bits */ #define VIRTIO_SCSI_F_INOUT0 #define VIRTIO_SCSI_F_HOTPLUG 1 +#define VIRTIO_SCSI_F_CHANGE 2 /* Response codes */ #define VIRTIO_SCSI_S_OK 0 @@ -63,6 +64,7 @@ #define VIRTIO_SCSI_T_NO_EVENT 0 #define VIRTIO_SCSI_T_TRANSPORT_RESET 1 #define VIRTIO_SCSI_T_ASYNC_NOTIFY 2 +#define VIRTIO_SCSI_T_PARAM_CHANGE 3 /* Reasons for transport reset event */ #define VIRTIO_SCSI_EVT_RESET_HARD 0 @@ -554,6 +556,7 @@ static uint32_t virtio_scsi_get_features(VirtIODevice *vdev, uint32_t requested_features) { requested_features |= (1UL VIRTIO_SCSI_F_HOTPLUG); +requested_features |= (1UL VIRTIO_SCSI_F_CHANGE); return requested_features; } @@ -637,6 +640,18 @@ static void virtio_scsi_handle_event(VirtIODevice *vdev, VirtQueue *vq) } } +static void virtio_scsi_change(SCSIBus *bus, SCSIDevice *dev, SCSISense sense) +{ +VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus); + +if (((s-vdev.guest_features VIRTIO_SCSI_F_CHANGE) 1) +(s-vdev.status VIRTIO_CONFIG_S_DRIVER_OK) +dev-type != TYPE_ROM) { +virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_PARAM_CHANGE, + sense.asc | (sense.ascq 8)); +} +} + static void virtio_scsi_hotplug(SCSIBus *bus, SCSIDevice *dev) { VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus); @@ -666,6 +681,7 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = { .complete = virtio_scsi_command_complete, .cancel = virtio_scsi_request_cancelled, +.change = virtio_scsi_change, .hotplug = virtio_scsi_hotplug, .hot_unplug = virtio_scsi_hot_unplug, .get_sg_list = virtio_scsi_get_sg_list, -- 1.7.10.4
[Qemu-devel] [PATCH 2/5] scsi-disk: report resized disk via sense codes
Linux will not use these, but a very similar mechanism will be used to report the condition via virtio-scsi events. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- hw/scsi-bus.c |5 + hw/scsi-disk.c | 15 +++ hw/scsi.h |2 ++ 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c index 8b961f2..2547c50 100644 --- a/hw/scsi-bus.c +++ b/hw/scsi-bus.c @@ -1161,6 +1161,11 @@ const struct SCSISense sense_code_LUN_FAILURE = { .key = ABORTED_COMMAND, .asc = 0x3e, .ascq = 0x01 }; +/* Unit attention, Capacity data has changed */ +const struct SCSISense sense_code_CAPACITY_CHANGED = { +.key = UNIT_ATTENTION, .asc = 0x2a, .ascq = 0x09 +}; + /* Unit attention, Power on, reset or bus device reset occurred */ const struct SCSISense sense_code_RESET = { .key = UNIT_ATTENTION, .asc = 0x29, .ascq = 0x00 diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 42bae3b..0905446 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -1863,6 +1863,13 @@ static void scsi_destroy(SCSIDevice *dev) blockdev_mark_auto_del(s-qdev.conf.bs); } +static void scsi_disk_resize_cb(void *opaque) +{ +SCSIDiskState *s = opaque; + +scsi_device_set_ua(s-qdev, SENSE_CODE(CAPACITY_CHANGED)); +} + static void scsi_cd_change_media_cb(void *opaque, bool load) { SCSIDiskState *s = opaque; @@ -1904,11 +1911,13 @@ static bool scsi_cd_is_medium_locked(void *opaque) return ((SCSIDiskState *)opaque)-tray_locked; } -static const BlockDevOps scsi_cd_block_ops = { +static const BlockDevOps scsi_disk_block_ops = { .change_media_cb = scsi_cd_change_media_cb, .eject_request_cb = scsi_cd_eject_request_cb, .is_tray_open = scsi_cd_is_tray_open, .is_medium_locked = scsi_cd_is_medium_locked, + +.resize_cb = scsi_disk_resize_cb, }; static void scsi_disk_unit_attention_reported(SCSIDevice *dev) @@ -1956,9 +1965,7 @@ static int scsi_initfn(SCSIDevice *dev) return -1; } -if (s-features (1 SCSI_DISK_F_REMOVABLE)) { -bdrv_set_dev_ops(s-qdev.conf.bs, scsi_cd_block_ops, s); -} +bdrv_set_dev_ops(s-qdev.conf.bs, scsi_disk_block_ops, s); bdrv_set_buffer_alignment(s-qdev.conf.bs, s-qdev.blocksize); bdrv_iostatus_enable(s-qdev.conf.bs); diff --git a/hw/scsi.h b/hw/scsi.h index 47c3b9c..e901350 100644 --- a/hw/scsi.h +++ b/hw/scsi.h @@ -198,6 +198,8 @@ extern const struct SCSISense sense_code_IO_ERROR; extern const struct SCSISense sense_code_I_T_NEXUS_LOSS; /* Command aborted, Logical Unit failure */ extern const struct SCSISense sense_code_LUN_FAILURE; +/* LUN not ready, Capacity data has changed */ +extern const struct SCSISense sense_code_CAPACITY_CHANGED; /* LUN not ready, Medium not present */ extern const struct SCSISense sense_code_UNIT_ATTENTION_NO_MEDIUM; /* Unit attention, Power on, reset or bus device reset occurred */ -- 1.7.10.4
[Qemu-devel] [PATCH 1/5] scsi-disk: removable hard disks support START/STOP
Support for START/STOP UNIT right now is limited to CD-ROMs. This is wrong, since removable hard disks (in the real world: SD card readers) also support it in pretty much the same way. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- hw/scsi-disk.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index bcec66b..42bae3b 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -1251,7 +1251,7 @@ static int scsi_disk_emulate_start_stop(SCSIDiskReq *r) bool start = req-cmd.buf[4] 1; bool loej = req-cmd.buf[4] 2; /* load on start, eject on !start */ -if (s-qdev.type == TYPE_ROM loej) { +if ((s-features (1 SCSI_DISK_F_REMOVABLE)) loej) { if (!start !s-tray_open s-tray_locked) { scsi_check_condition(r, bdrv_is_inserted(s-qdev.conf.bs) -- 1.7.10.4
Re: [Qemu-devel] qemu fails to build with glibc-2.15
(fixed mailing list) On 07/16/2012 03:37 PM, X O wrote: Hello, I suspect upgrading my system to glibc-2.15 was a mistake. It seems to be qemu-1.0.1, and latter versions including qemu-1.1.1, can't be compiled anymore. Yes, I did search around and that led me to glibc, resp. http://sourceware.org/ml/libc-ports/2011-08/msg00019.html Please, could somebody confirm or deny the following error is thanks to glibc-2.15? Is there a way to compile qemu with glibc-2.15? Or is my system broken? ~~~ CCi386-linux-user/syscall.o /tmp/qemu-1.1.1/linux-user/syscall.c: In function 'sys_getdents': /tmp/qemu-1.1.1/linux-user/syscall.c:217:1: error: '__NR_getdents' undeclared (first use in this function) /tmp/qemu-1.1.1/linux-user/syscall.c:217:1: note: each undeclared identifier is reported only once for each function it appears in /tmp/qemu-1.1.1/linux-user/syscall.c: In function '_llseek': /tmp/qemu-1.1.1/linux-user/syscall.c:224:1: error: '__NR_lseek' undeclared (first use in this function) Please report the output of grep -r __NR_getdents /usr/include/ on your system. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH v3 2/2] bitops: fix types
On 07/14/2012 03:34 PM, Blue Swirl wrote: bitops.h uses inconsistently 'unsigned long' and 'int' for bit numbers. Unify to 'unsigned long' because it generates better code on x86_64. Adjust asserts accordingly. Actually, plain unsigned generates the best code. unsigned longs require an extra byte for many instructions. It also requires more stack space if spilled. Unsigned does not, but is compatible with unsigned long in case it needs to be used in pointer arithmetic (unlike signed int which needs a sign extension instruction). -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH v2] qerror: Add QERR_PROPERTY_SET_AFTER_REALIZE
Add a new QError QERR_PROPERTY_SET_AFTER_REALIZE for attempts to set a QOM or qdev property after the object/device has been realized. This allows a slightly more informative diagnostic than the previous permission denied message. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- Changes since the v1 (which was sent way back in March...): * rebased on master now a pile of qdev/qom changesd have landed * fixed some overlong lines * avoid gcc '?:' extension * a couple of set_ functions in qdev-properties.c are new since v1 and needed their QERR_PERMISSION_DENIED checks changing hw/qdev-properties.c | 42 -- qerror.c |5 + qerror.h |3 +++ 3 files changed, 36 insertions(+), 14 deletions(-) diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 0b89462..ae0c7a7 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -54,7 +54,8 @@ static void set_bit(Object *obj, Visitor *v, void *opaque, bool value; if (dev-state != DEV_STATE_CREATED) { -error_set(errp, QERR_PERMISSION_DENIED); +error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, + dev-id ? dev-id : , name); return; } @@ -94,7 +95,8 @@ static void set_uint8(Object *obj, Visitor *v, void *opaque, uint8_t *ptr = qdev_get_prop_ptr(dev, prop); if (dev-state != DEV_STATE_CREATED) { -error_set(errp, QERR_PERMISSION_DENIED); +error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, + dev-id ? dev-id : , name); return; } @@ -161,7 +163,8 @@ static void set_uint16(Object *obj, Visitor *v, void *opaque, uint16_t *ptr = qdev_get_prop_ptr(dev, prop); if (dev-state != DEV_STATE_CREATED) { -error_set(errp, QERR_PERMISSION_DENIED); +error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, + dev-id ? dev-id : , name); return; } @@ -194,7 +197,8 @@ static void set_uint32(Object *obj, Visitor *v, void *opaque, uint32_t *ptr = qdev_get_prop_ptr(dev, prop); if (dev-state != DEV_STATE_CREATED) { -error_set(errp, QERR_PERMISSION_DENIED); +error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, + dev-id ? dev-id : , name); return; } @@ -219,7 +223,8 @@ static void set_int32(Object *obj, Visitor *v, void *opaque, int32_t *ptr = qdev_get_prop_ptr(dev, prop); if (dev-state != DEV_STATE_CREATED) { -error_set(errp, QERR_PERMISSION_DENIED); +error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, + dev-id ? dev-id : , name); return; } @@ -292,7 +297,8 @@ static void set_uint64(Object *obj, Visitor *v, void *opaque, uint64_t *ptr = qdev_get_prop_ptr(dev, prop); if (dev-state != DEV_STATE_CREATED) { -error_set(errp, QERR_PERMISSION_DENIED); +error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, + dev-id ? dev-id : , name); return; } @@ -380,7 +386,8 @@ static void set_string(Object *obj, Visitor *v, void *opaque, char *str; if (dev-state != DEV_STATE_CREATED) { -error_set(errp, QERR_PERMISSION_DENIED); +error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, + dev-id ? dev-id : , name); return; } @@ -458,7 +465,8 @@ static void set_pointer(Object *obj, Visitor *v, Property *prop, int ret; if (dev-state != DEV_STATE_CREATED) { -error_set(errp, QERR_PERMISSION_DENIED); +error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, + dev-id ? dev-id : , name); return; } @@ -627,7 +635,8 @@ static void set_vlan(Object *obj, Visitor *v, void *opaque, VLANState *vlan; if (dev-state != DEV_STATE_CREATED) { -error_set(errp, QERR_PERMISSION_DENIED); +error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, + dev-id ? dev-id : , name); return; } @@ -697,7 +706,8 @@ static void set_mac(Object *obj, Visitor *v, void *opaque, char *str, *p; if (dev-state != DEV_STATE_CREATED) { -error_set(errp, QERR_PERMISSION_DENIED); +error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, + dev-id ? dev-id : , name); return; } @@ -767,7 +777,8 @@ static void set_enum(Object *obj, Visitor *v, void *opaque, int *ptr = qdev_get_prop_ptr(dev, prop); if (dev-state != DEV_STATE_CREATED) { -error_set(errp, QERR_PERMISSION_DENIED); +error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, + dev-id ? dev-id : , name); return; } @@ -798,7 +809,8 @@ static void set_pci_devfn(Object *obj, Visitor *v, void *opaque, char *str; if (dev-state != DEV_STATE_CREATED) { -error_set(errp, QERR_PERMISSION_DENIED); +error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE, + dev-id ? dev-id : , name);
Re: [Qemu-devel] Qemu Live Migration: File Descriptor
On 07/14/2012 08:45 AM, siddharth srivastava wrote: Hi I have been exploring various live migration ways that qemu supports. In the Qemu Migration documentation at: [1], it is only mentioned how to invoke tcp and exec migration schemes. But there is no mention of unix and fd migration schemes. In the code I see that the argument for tcp, fd,unix and exec goes into uri. But neither in the code nor in the docs/migration, it is mentioned what it expects as uri. So how do you start migration with these schemes ? The libvirt code base exposes all of these migration schemes (although it currently favors fd: migration wherever possible). Starting a migration requires a Unix socket on the monitor command, and passing in the file descriptor via SCM_RIGHTS via the 'getfd' monitor command prior to using the 'migrate' monitor command to use that new fd. -- 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 v3 2/2] bitops: fix types
On 14 July 2012 13:34, Blue Swirl blauwir...@gmail.com wrote: bitops.h uses inconsistently 'unsigned long' and 'int' for bit numbers. Unify to 'unsigned long' because it generates better code on x86_64. Adjust asserts accordingly. Still disagree with this patch, for the record. -- PMM
Re: [Qemu-devel] [PATCH] configure: Fix build with capabilities
On 07/15/2012 07:54 AM, Stefan Weil wrote: Since commit 417c9d72d48275d19c60861896efd4962d21aca2 all configure tests normally run with -Werror. Some of these tests now fail because they raised a compiler warning. This patch fixes support for capabilities. Signed-off-by: Stefan Weil s...@weilnetz.de --- configure |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index f745cc2..abde4c0 100755 --- a/configure +++ b/configure @@ -2082,7 +2082,7 @@ if test $cap != no ; then cat $TMPC EOF #include stdio.h #include sys/capability.h -int main(void) { cap_t caps; caps = cap_init(); } +int main(void) { cap_t caps; caps = cap_init(); return caps != NULL; } Nothing wrong with this patch, per se, but in the Autoconf world, the general advice is that programs should check whether -Werror is supported, but then avoid using it for the entire remainder of the configure script (that is, store the result of -Werror into a different variable that gets added into the final CFLAGS at make time, but not used during any of the rest of the configure time). That's because it's just too hard to avoid warnings-turned-into-errors for all possible versions (including future releases) of gcc, so you are just too likely to run into spurious changes in configurations when the next version of gcc starts warning about something new if you try to run all your configure tests with -Werror. -- 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] configure: do not quote $PKG_CONFIG
On 07/15/2012 01:54 PM, Stefan Weil wrote: Am 15.07.2012 22:26, schrieb Mike Frysinger: We should not quote the PKG_CONFIG setting as this deviates from the canonical upstream behavior that gets integrated with all other build systems, and deviates from how we treat all other toolchain variables that we get from the environment. Ultimately, the point is that it breaks passing custom flags directly to pkg-config via the env var where this normally works elsewhere, and it used to work in the past. What about passing custom flags with QEMU_PKG_CONFIG_FLAGS? Removing the quotes will not allow paths containing spaces, so that's not a good idea. Actually, it IS a good idea. The de facto standard build environment requires that pkg-config is not allowed to live in a path containing spaces, precisely so that you can override the variable to pass options to your preferred location of pkg-config; and if your build setup is truly so messed up as to have pkg-config installed in a canonical location with spaces, then you can also tweak your unusual environment to provide a symlink to pkg-config that does not contain spaces as the workaround. -- 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] configure: Fix build with capabilities
On 16 July 2012 16:35, Eric Blake ebl...@redhat.com wrote: On 07/15/2012 07:54 AM, Stefan Weil wrote: Since commit 417c9d72d48275d19c60861896efd4962d21aca2 all configure tests normally run with -Werror. Some of these tests now fail because they raised a compiler warning. Nothing wrong with this patch, per se, but in the Autoconf world, the general advice is that programs should check whether -Werror is supported, but then avoid using it for the entire remainder of the configure script (that is, store the result of -Werror into a different variable that gets added into the final CFLAGS at make time, but not used during any of the rest of the configure time). That's because it's just too hard to avoid warnings-turned-into-errors for all possible versions (including future releases) of gcc, so you are just too likely to run into spurious changes in configurations when the next version of gcc starts warning about something new if you try to run all your configure tests with -Werror. Hmm, good point -- unlike the actual program compile, failures due to -Werror inside configure are silent and near-invisible. Maybe we should back out 417c9d72 ? -- PMM
Re: [Qemu-devel] [PATCH 01/16] smbios: Add a function to directly add an entry
On 07/15/2012 02:24 PM, miny...@acm.org wrote: From: Corey Minyard cminy...@mvista.com There was no way to directly add a table entry to the SMBIOS table, even though the BIOS supports this. So add a function to do this. This is in preparation for the IPMI handler adding it's SMBIOS table entry. Signed-off-by: Corey Minyard cminy...@mvista.com --- hw/smbios.c | 27 +++ hw/smbios.h | 15 --- 2 files changed, 35 insertions(+), 7 deletions(-) Are you planning to expose this via the command line? Libvirt would like the ability to set arbitrary name/value pairs in SMBIOS (such as asset tag in the type 3 page), rather than the current limitation of only being able to set pre-defined names in just type 0 and 1 sections. -- 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] configure: do not quote $PKG_CONFIG
Am 16.07.2012 17:39, schrieb Eric Blake: On 07/15/2012 01:54 PM, Stefan Weil wrote: Am 15.07.2012 22:26, schrieb Mike Frysinger: We should not quote the PKG_CONFIG setting as this deviates from the canonical upstream behavior that gets integrated with all other build systems, and deviates from how we treat all other toolchain variables that we get from the environment. Ultimately, the point is that it breaks passing custom flags directly to pkg-config via the env var where this normally works elsewhere, and it used to work in the past. What about passing custom flags with QEMU_PKG_CONFIG_FLAGS? Removing the quotes will not allow paths containing spaces, so that's not a good idea. Actually, it IS a good idea. The de facto standard build environment requires that pkg-config is not allowed to live in a path containing spaces, precisely so that you can override the variable to pass options to your preferred location of pkg-config; and if your build setup is truly so messed up as to have pkg-config installed in a canonical location with spaces, then you can also tweak your unusual environment to provide a symlink to pkg-config that does not contain spaces as the workaround. That sounds reasonable. Then the following patch was at least partially unnecessary: commit 17884d7b6462b0fe497f08fec6091ffbe04caa8d Author: Sergei Trofimovich sly...@gentoo.org Date: Tue Jan 31 22:03:45 2012 +0300 ./configure: request pkg-config to provide private libs when static linking Added wrapper around pkg-config to allow: - safe options injection via ${QEMU_PKG_CONFIG_FLAGS} - spaces in path to pkg-config Signed-off-by: Sergei Trofimovich sly...@gentoo.org CC: Peter Maydell peter.mayd...@linaro.org Signed-off-by: Anthony Liguori aligu...@us.ibm.com With Mike's new patch, QEMU_PKG_CONFIG_FLAGS is no longer needed because options can be passed using the pkg-config macro. I suggest to remove it. Regards, Stefan W.
Re: [Qemu-devel] [PATCH v5 2/4] exynos4210: Added SD host controller model
On 5 July 2012 05:04, Peter A. G. Crosthwaite peter.crosthwa...@petalogix.com wrote: From: Igor Mitsyanko i.mitsya...@samsung.com Custom Exynos4210 SD/MMC host controller, based on SD association standard host controller ver. 2.00. Signed-off-by: Igor Mitsyanko i.mitsya...@samsung.com --- changed from v4 (Igor): set irq on SLOTINT status instead of interrupt registers status; instead; The IRQ handling code still looks really weird. I would expect that the code would be: [code which updates various kinds of irq related state] sdhci_update_irq(); where sdhci_update_irq() calls qemu_set_irq() based on the state. At the moment it looks as if you're using slotint as a cached value of the expression ((s-norintsts s-norintsigen) || (s-errintsts s-errintsigen) || ((s-norintsts SDHC_NIS_INSERT) (s-wakcon SDHC_WKUP_ON_INS)) || ((s-norintsts SDHC_NIS_REMOVE) (s-wakcon SDHC_WKUP_ON_RMV))) [can these two ever have different values?] and also attempting to shortcut by manually updating slotint in codepaths which change only parts of the state which this expression is testing. Why not just do things the simple and straightforward way and get rid of slotint completely? -- PMM
Re: [Qemu-devel] [PATCH] configure: Fix build with capabilities
On 07/16/2012 05:40 PM, Peter Maydell wrote: On 16 July 2012 16:35, Eric Blake ebl...@redhat.com wrote: On 07/15/2012 07:54 AM, Stefan Weil wrote: Since commit 417c9d72d48275d19c60861896efd4962d21aca2 all configure tests normally run with -Werror. Some of these tests now fail because they raised a compiler warning. Nothing wrong with this patch, per se, but in the Autoconf world, the general advice is that programs should check whether -Werror is supported, but then avoid using it for the entire remainder of the configure script (that is, store the result of -Werror into a different variable that gets added into the final CFLAGS at make time, but not used during any of the rest of the configure time). That's because it's just too hard to avoid warnings-turned-into-errors for all possible versions (including future releases) of gcc, so you are just too likely to run into spurious changes in configurations when the next version of gcc starts warning about something new if you try to run all your configure tests with -Werror. Hmm, good point -- unlike the actual program compile, failures due to -Werror inside configure are silent and near-invisible. Maybe we should back out 417c9d72 ? So how do we deal with the original problem then? The one where the build broke for me because the smartcard code got compiled in because configure didn't know that it would fail to compile with -Werror? Alex
Re: [Qemu-devel] [PATCH v5 3/4] vl.c: allow for repeated -sd arguments
On 5 July 2012 05:04, Peter A. G. Crosthwaite peter.crosthwa...@petalogix.com wrote: Allows for repeating of -sd arguments in the same way as -pflash and -mtdblock. Signed-off-by: Peter A. G. Crosthwaite peter.crosthwa...@petalogix.com Acked-by: Igor Mitsyanko i.mitsya...@samsung.com Reviewed-by: Peter Maydell peter.mayd...@linaro.org -- PMM
Re: [Qemu-devel] [PATCH v5 4/4] xilinx_zynq: Added SD controllers
On 5 July 2012 05:04, Peter A. G. Crosthwaite peter.crosthwa...@petalogix.com wrote: The Xilinx Zynq device has two SDHCI controllers. Added to the machine model. Signed-off-by: Peter A. G. Crosthwaite peter.crosthwa...@petalogix.com Reviewed-by: Peter Maydell peter.mayd...@linaro.org -- PMM