[Qemu-devel] KVM call agenda for Mar 23
Please send in any agenda items you are interested in covering. Yes, usability is a valid topic esp. if you promise to come w/ GUI patches. thanks, -chris
Re: [Qemu-devel] git head broken? (x86 softmmu w/o kvm)
On Mon, Mar 22, 2010 at 10:25:24PM +0100, Juergen Lock wrote: > Hi! > > I just wanted to make another FreeBSD qemu git head snaphot port update, > and found both i386-softmmu and x86_64-softmmu no longer boot, they seem > to hang early in the bios before it prints anything, last tb seems to be > this loop: > A quick bisect revealed it has been broken by this patch: commit 952760bb7bce7fbfe0afcf04fee268745f297b87 Author: Blue Swirl Date: Sun Mar 21 19:47:15 2010 + Compile pci_host only once Convert pci_host_conf_register_mmio_noswap(x) to pci_host_conf_register_mmio(x, 0). Convert pci_host_conf_register_mmio(x) to pci_host_conf_register_mmio(x, 1) for big endian hosts, all cases happen to be BE. Signed-off-by: Blue Swirl -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups
On 03/22/2010 07:49 PM, Paul Brook wrote: Solutions: - VirtIOPCIBus and hang devices from there (anthony). Why? because this is a simulated pci bus, we can implement the features that we need (not full pci) in the three showed architectures. We will have VirtIOPCIBLock everywhere, and its VirtIOPCIBus implmentation will hide the details. This is not how I understood Anthony's proposal. VirtIOPCIBus makes no sense. The whole reason we have this problem is because the VirtIO devices can not make any assumptions about the bus they are connected to. The key point is that we promote VirtIO devices to nodes in the device tree. i.e. VirtIODevice descends directly from DeviceState. Instead of trying to make a single polymorphic hybryd object, split into separate objects for the component parts. Each host binding (PCI, syborg, s390, etc.) provides a single VirtIO bridge device. This includes a VirtIOBus, to which the VirtIODevice is attached. Most of the code and abstraction layers for this are already there. We just replace virtio_bind_device with VirtIOBus, add direct registration of VirtIODevice, and rip out all the crufty old device specific bits from virtio- pci.c. Right. The only real challenge is dealing with legacy save/restore and command line syntax. For save/restore, we can possibly have a dummy device that can split the VirtioPCI device state from the virtio device states and do the right thing. I'm not sure what we should do for command line syntax. We can keep -drive working as is. Do we need to support -device based creation? I don't think we've really considered what to do in a situation like this. Regards, Anthony Liguori - having multiple inheritance is "more natural" to virtio, then we have to change all the system to be able to allow multiple inherintance, even if it is more complex, and nothing else uses/needs it (mst). I'm not convinced multiple inheritance solves the real problem, much less that it is worthwhile. Paul
Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups
On 03/22/2010 04:00 PM, Paul Brook wrote: On 03/22/2010 11:16 AM, Paul Brook wrote: But look at the lguest virtio implement. We would definitely model a VirtIOBus if we implemented something like that in qemu. VirtIO really is designed to be a bus. When you say "bus" you actually mean point-point connection, right[1]? I don't see anything in virtio that allows arbitration of multiple devices, or any particular need for one as it can be handled by the host bus bindings. Virtio itself doesn't define any type of bus operations but is designed to let it nicely fit into existing bus infrastructures. My understanding is that virtio specifies transport agnostic queueing API for passing buffers of data, and a small device specific config space. IMO a qemu virtio bus should implement exactly that. The host bridge submits buffers, and the device processes them. Allowing more than one device on this bus just adds complication for no benefit. The only reason to have multiple devices on a single bus is so that the can arbitrate a shared resource, and in this case we have no resources to share. If a single host device wants to support multiple virtio devices then it can create multiple busses. The purpose of the virtio bus is to expose the isolation between virtio device and host bridge/binding to the user. This allows virtio devices to be configured consistently without having to duplicate N hosts * M devices. I think we ultimately agree. I think your point is that it's not technically a bus but it's convenient to model it that way. I definitely understand that point. Regards, Anthony Liguori
Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups
>Solutions: >- VirtIOPCIBus and hang devices from there (anthony). Why? because > this is a simulated pci bus, we can implement the features that we > need (not full pci) in the three showed architectures. We will have > VirtIOPCIBLock everywhere, and its VirtIOPCIBus implmentation will > hide the details. This is not how I understood Anthony's proposal. VirtIOPCIBus makes no sense. The whole reason we have this problem is because the VirtIO devices can not make any assumptions about the bus they are connected to. The key point is that we promote VirtIO devices to nodes in the device tree. i.e. VirtIODevice descends directly from DeviceState. Instead of trying to make a single polymorphic hybryd object, split into separate objects for the component parts. Each host binding (PCI, syborg, s390, etc.) provides a single VirtIO bridge device. This includes a VirtIOBus, to which the VirtIODevice is attached. Most of the code and abstraction layers for this are already there. We just replace virtio_bind_device with VirtIOBus, add direct registration of VirtIODevice, and rip out all the crufty old device specific bits from virtio- pci.c. > - having multiple inheritance is "more natural" to virtio, then we have > to change all the system to be able to allow multiple inherintance, > even if it is more complex, and nothing else uses/needs it (mst). I'm not convinced multiple inheritance solves the real problem, much less that it is worthwhile. Paul
Re: [Qemu-devel] Re: [libvirt] Supporting hypervisor specific APIs in libvirt
On 03/22/2010 05:33 PM, Gerd Hoffmann wrote: > Hi, > >> Stepping back a bit first, there are the two core areas in which >> people can >> be limited by libvirt currently. > >> 2. Command line flags > > For me: This one, and monitor access. > > libvirt is very unfriendly to qemu hackers. There is no easy way to add > command line switches. There is no easy way to get access to the > monitor. I can get it done by pointing to a wrapper script > and mangle the qemu command line there. But this sucks big time. And > it doesn't integrate with libvirt at all. > > When hacking qemu, especially when adding new command line options or > monitor commands, I want to have a easy way to test this stuff. Or I > just wanna able to type some 'info $foo' commands for debugging and > trouble shooting purposes. libvirt makes it harder not easier to get > the job done. > > Image you could ask libvirt to create an additional monitor and expose > it like a serial console. virt-manager lists it as text console. Two > mouse clicks open a new window (or tab) with a terminal emulator linked > to the monitor. Wouldn't that be cool? > > Other issues I've trap into: > > -boot > libvirt (or virt-manager?) supports only the very old single letter > style. You can't specify '-boot order=cd,menu=on'. > Libvirt has supported multiple boot options for a while, it just wasn't in virt-manager. It's been upstream for a few weeks now though, and a new release is coming in a matter of days. I have a half implemented libvirt patch to allow setting boot menu, I guess it's time to dust it off :) > -enable-nested > not available. > > serial console doesn't work for remote connections. > Both of these have been requested a few times, so you aren't alone. - Cole
[Qemu-devel] Re: [PATCH 0/9] Virtio cleanups
"Michael S. Tsirkin" wrote:
> On Mon, Mar 22, 2010 at 03:51:43PM +, Paul Brook wrote:
>> > > It's a classic OOP problem.
>> > >
>> > > VirtIOBlock is-a VirtIODevice, VirtIODevice is-a DeviceState
>> > >
>> > > VirtIOPCI is-a PCIDevice, PCIDevice is-a Device State.
>> > >
>> > > But VirtIODevice is-a VirtIOPCI device isn't always true so it can't be
>> > > an is-a relationship. Initially, this was true and that's why the code
>> > > was structured that way. Now that we have two type so of virtio
>> > > transports, we need to change the modelling. It needs to get inverted
>> > > into a has-a relationship. IOW, VirtIOPCI has-a VirtIODevice.
>> > >
>> > > When one device has-a one or more other devices, we model that as a Bus.
>> >
>> > Hmm. Is anything wrong with VirtIOPCIBlock which would be both a
>> > VirtIOBlock and VirtIOPCI device?
>>
>> You need to solve multiple inheritance with common (but divergent) ancestors.
>>
>> A single device may not have more than one DeviceState. i.e. the DeviceState
>> that is part of the VirtIOBlock must be the same as the DeviceState that is
>> part of the PCIDevice. However VirtIOBlock and PCIDevice can not know about
>> each other or about VirtIOPCIBlock.
>>
>> In the current code VirtIOPCIBlock already exists, though only for the
>> purposes of device creation/configuration. The remainder of the code and
>> data
>> structures have clean separation between PCI and Virtio code.
>
> We can have VirtIOPCIBlock have a single DeviceState.
How?
That is the real problem he are having here.
struct VirtIODevice
{
. No DeviceState at all
};
typedef struct VirtIOBlock
{
VirtIODevice vdev;
} VirtIOBlock;
typedef struct {
PCIDevice pci_dev;
VirtIODevice *vdev;
} VirtIOPCIProxy;
Not relevant fields for the discussion removed. Virtio-blk taken as an
example, but virtio-net/balloon/serial has the same structure.
Some virtio-pci functions (chosen to not be a vmstate one, but there are
more with this same structure).
static void virtio_pci_reset(DeviceState *d) (*)
{
VirtIOPCIProxy *proxy = container_of(d, VirtIOPCIProxy, pci_dev.qdev);
virtio_reset(proxy->vdev);
msix_reset(&proxy->pci_dev);
}
We have a something that cames somehow from qdev or PCIDevice and we had
to also do something on the vdev.
Current situation:
- we have "sub-classes" of VirtIODevice (VirtIOBlock in this case).
Everything outside of virtio-blk only cares for VirtIODevice fields.
They are common for ol VirtIO* devices.
Moving VirtIODevice out of offset 0 brings us nothing, until this
reqeriment is removed.
- How do we arrive here? We have three "kinds" of virtio devices:
- pci ones (kvm)
- sysborg (not pci)
- s390
And they share the code except for how to access the bus.
Solutions:
- VirtIOPCIBus and hang devices from there (anthony). Why? because
this is a simulated pci bus, we can implement the features that we
need (not full pci) in the three showed architectures. We will have
VirtIOPCIBLock everywhere, and its VirtIOPCIBus implmentation will
hide the details. Notice that this will make VirtIODevices more
similar to the other kind of devices (see LSI example from Anthony in
other email in the thread, see also mst answer that VirtIO is not like
scsi either for the other point of view).
- Create something like:
struct VirtIOPCIBlock {
PCIDevice pci_dev;
VirtIODevice vdev;
}
this make lots of things easier, but makes impossible/very difficult
to: - share virtio-pci code (now we dont' have a common layaut
struct).
mst suggestion is to have something like:
struct VirtIOPCIBlock {
PCIDevice pci_dev;
VirtIODevice *vdev;
struct VirtIOBlock block;
}
and having vdev point to the right place. The same for all devices.
My question is how _requiring a zero_ offset for vdev inside
VirtIOBlock is ugly, but having this vdev pointer to an internal part
of the same struct is more elegant (in my book it is worse).
- Just replicate virtio-pci.c for each VirtIO* device and change the
types accordingly. This is way clearer in the sense of not having
pointers to inside the same struct, but it has obvious maintenance
nightmares.
If there are any better ideas around, I haven't seen them yet :-(
As Anthony described it: it is the best of two devils. My point to not
like the mst change: I really think that it would never be used, it is
just over-engenieering. Why do I think that? Because something like
Anthony suggestion (VirtPCIBlock) fits so much better with qemu/qdev
device model. Having that a VirtPCIBlock is-a VirtPCIProxy and a
VirtIODevice don't fit with the qemu model, I haven't seen any other
device that uses _double inheritance_. And here is where the matter of
the discussion with mst appears:
- having single inheritance makes everything simpler -> virtio has to
adapt to single inheritance (me)
- having multiple inheritance is "more
Re: [Qemu-devel] Re: [libvirt] Supporting hypervisor specific APIs in libvirt
On 03/22/2010 04:33 PM, Gerd Hoffmann wrote: Hi, Stepping back a bit first, there are the two core areas in which people can be limited by libvirt currently. 2. Command line flags For me: This one, and monitor access. libvirt is very unfriendly to qemu hackers. There is no easy way to add command line switches. There is no easy way to get access to the monitor. I can get it done by pointing to a wrapper script and mangle the qemu command line there. But this sucks big time. And it doesn't integrate with libvirt at all. It's not just developers. As we're doing deployments of qemu/kvm, we keep running into the same problem. We realize that we need to use a feature of qemu/kvm that isn't modelled by libvirt today. I've gone as far as to temporarily pausing libvirtd, finding the pty fd from /proc/, and hijacking the monitor session temporarily. The problem is, it's not always easy to know what the most important features are. When hacking qemu, especially when adding new command line options or monitor commands, I want to have a easy way to test this stuff. Or I just wanna able to type some 'info $foo' commands for debugging and trouble shooting purposes. libvirt makes it harder not easier to get the job done. Image you could ask libvirt to create an additional monitor and expose it like a serial console. virt-manager lists it as text console. Two mouse clicks open a new window (or tab) with a terminal emulator linked to the monitor. Wouldn't that be cool? Other issues I've trap into: -boot libvirt (or virt-manager?) supports only the very old single letter style. You can't specify '-boot order=cd,menu=on'. You can, you specify multiple options (but you can't touch things like menu=on). Regards, Anthony Liguori -enable-nested not available. serial console doesn't work for remote connections. cheers, Gerd
[Qemu-devel] Re: [libvirt] Supporting hypervisor specific APIs in libvirt
On 03/22/2010 03:10 PM, Daniel P. Berrange wrote: This isn't necessarily libvirt's problem if it's mission is to provide a common hypervisor API that covers the most commonly used features. That is more or less our current mission. If this mission leads to QEMU creating a non-libvirt based API& telling people to use that instead, then I'd say libvirt's mission needs to change to avoid that scenario ! I strongly believe that libvirt's strategy is good for application developers over the medium to long term. We need to figure out how to get rid of the short term pain from the feature timelag, rather than inventing a new library API for them to use. Well that's certainly a good thing :-) However, for qemu, we need an API that covers all of our features that people can develop against. The ultimate question we need to figure out is, should we encourage our users to always use libvirt or should we build our own API for people (and libvirt) to consume. I don't think it's necessarily a big technical challenge for libvirt to support qemu more completely. I think it amounts to introducing a series of virQemu APIs that implement qemu specific functions. Over time, qemu specific APIs can be deprecated in favour of more generic virDomain APIs. Stepping back a bit first, there are the two core areas in which people can be limited by libvirt currently. 1. Monitor commands 2. Command line flags Ultimately, IIUC, you are suggesting we need to allow arbitrary passthrough for both of these in libvirt. At the libvirt level, we have 3 core requirements 1. The XML format is extend only (new elements allowed, or add attributes or children to existing elements) 2. The C library API is append only (new symbols only) 3. The RPC wire protocol is append only (maps 1-1 to the C API generally) We have a slightly different mentality within QEMU I think. Here's roughly how I'd characterize our guarantees. 1. For any two versions of QEMU, we try to guarantee that the same VM, as far as the guest sees it, can be created. 2. We tend to avoid changing command line syntax unless the syntax was previously undefined. 3. QMP supports enumeration and feature negotiation. This enables a client to discover which functions are supported. 4. We try to maintain monitor interfaces but provide no guarantees of compatibility. The core question for us as libvirt developers is how we could support QEMU specific features that may change arbitrarily, without it impacting on our ability to maintain these 3 requirements for the non-hypervisor specific APIs. We don't ever want to be in a situation where a QEMU specific API will require us to change the soname of the main libvirt library, or introduce incompatible wire protocol changes. If we were to introduce QEMU specific APIs, we also need a way to easily remove those over time, as& when we have them available as generic APIs. At the C API level, this to me suggests that we'd want to introduce a separate libvirt-qemu.so library for the QEMU specific APIs. This library would not have the same requirements of fixed long term ABI that the main libvirt.so did. We'd add QEMU APIs to libvirt-qemu.so any time needed, but remove them when the equivalent functionality were in libvirt.so, and increment the soname of libvirt-qemu.so at that point. How different is having a libvirt-qemu.so from having a libqemu.so that libvirt.so uses? Practically speaking, if libvirt-qemu.so uses a separate XML namespace, does the fact that we use a different config format matter since you can transform our config format to XML and vice versa? I think the problem is, if libvirt.so introduces a common API, removing it from libvirt-qemu.so is burdensome to an end-user. For someone designing a QEMU specific management application, why should they have to update their implementation to a common API that they'll never use? At the wire protocol level, the protocol allows us to support multiple versioned protocols in parallel over the same data stream. So again there, we could define a sub-protocol for QEMU specific features for which we don't provide the indefinite ABI compatability. Finally the XML format is "easy" - just have a versioned XML namespace for extra pieces, that's distinct from the default namespace, again without the permanent long term compatability guarentees. There are, however, some bits that are unlikely to work when QEMU is under libvirt. Specifically any of the device backends that use stdio (eg, -serial stdio, or the ncurses graphics), simply because all libvirt spawned VMs are fully daemonized& so stdio is /dev/null This seems fixable with //session. Other items are hard, but not entirely impossible to solve. eg, any use of the 'script=' arg for -net devices doesn't work, because libvirt clears all capabilities from the QEMU process so it'll be lacking CAP_NET_ADMIN which most TAP device setup scripts in fact need.
[Qemu-devel] Re: git head broken? (x86 softmmu w/o kvm)
On Mon, Mar 22, 2010 at 10:25:24PM +0100, Juergen Lock wrote: > Hi! > > I just wanted to make another FreeBSD qemu git head snaphot port update, > and found both i386-softmmu and x86_64-softmmu no longer boot, they seem > to hang early in the bios before it prints anything, last tb seems to be > this loop: > > > IN: > 0x000f1b8e: mov0xf81a0,%ecx > 0x000f1b94: cmp%ecx,%eax > 0x000f1b96: jne0xf1b8e > > OUT: [size=184] > 0x4000e440: mov$0xf81a0,%ebp > 0x4000e445: mov%rbp,%rsi > 0x4000e448: mov%rbp,%rdi > 0x4000e44b: shr$0x7,%rsi > 0x4000e44f: and$0xf003,%rdi > 0x4000e456: and$0x1fe0,%esi > 0x4000e45c: lea0x4f8(%rsi,%r14,1),%rsi > 0x4000e464: cmp(%rsi),%rdi > 0x4000e467: mov%rbp,%rdi > 0x4000e46a: je 0x4000e477 > 0x4000e46c: xor%esi,%esi > 0x4000e46e: callq 0x51fd30 > 0x4000e473: mov%eax,%ebp > 0x4000e475: jmp0x4000e47d > 0x4000e477: add0x18(%rsi),%rdi > 0x4000e47b: mov(%rdi),%ebp > 0x4000e47d: mov%ebp,%ebp > 0x4000e47f: mov%rbp,%rbx > 0x4000e482: mov(%r14),%r12 > 0x4000e485: mov%rbx,%r13 > 0x4000e488: sub%rbx,%r12 > 0x4000e48b: mov%r12,%rbx > 0x4000e48e: mov%ebx,%ebx > 0x4000e490: mov$0x10,%r15d > 0x4000e496: mov%r15d,0xa0(%r14) > 0x4000e49d: mov%r13,0x90(%r14) > 0x4000e4a4: mov%r12,0x98(%r14) > 0x4000e4ab: mov%rbp,0x8(%r14) > 0x4000e4af: test %rbx,%rbx > 0x4000e4b2: jne0x4000e4d8 > 0x4000e4b8: jmpq 0x4000e4bd > 0x4000e4bd: mov$0xf1b98,%ebp > 0x4000e4c2: mov%rbp,0x80(%r14) > 0x4000e4c9: mov$0x802c05c80,%rax > 0x4000e4d3: jmpq 0xb65b8e > 0x4000e4d8: jmpq 0x4000e4dd > 0x4000e4dd: mov$0xf1b8e,%ebp > 0x4000e4e2: mov%rbp,0x80(%r14) > 0x4000e4e9: mov$0x802c05c81,%rax > 0x4000e4f3: jmpq 0xb65b8e > > Is 0xf81a0 an io port or how is it supposed to change? And, can > anyone reproduce this on Linux? As I said this is without kvm... ..and in case its supposed to be changed by an irq I just tried -d in_asm,out_asm,int and saw none listed. I've put the qemu.log here: http://people.freebsd.org/~nox/qemu/qemu.log.gz TIA, Juergen
Re: [Qemu-devel] Re: [libvirt] Supporting hypervisor specific APIs in libvirt
Hi, Stepping back a bit first, there are the two core areas in which people can be limited by libvirt currently. 2. Command line flags For me: This one, and monitor access. libvirt is very unfriendly to qemu hackers. There is no easy way to add command line switches. There is no easy way to get access to the monitor. I can get it done by pointing to a wrapper script and mangle the qemu command line there. But this sucks big time. And it doesn't integrate with libvirt at all. When hacking qemu, especially when adding new command line options or monitor commands, I want to have a easy way to test this stuff. Or I just wanna able to type some 'info $foo' commands for debugging and trouble shooting purposes. libvirt makes it harder not easier to get the job done. Image you could ask libvirt to create an additional monitor and expose it like a serial console. virt-manager lists it as text console. Two mouse clicks open a new window (or tab) with a terminal emulator linked to the monitor. Wouldn't that be cool? Other issues I've trap into: -boot libvirt (or virt-manager?) supports only the very old single letter style. You can't specify '-boot order=cd,menu=on'. -enable-nested not available. serial console doesn't work for remote connections. cheers, Gerd
[Qemu-devel] git head broken? (x86 softmmu w/o kvm)
Hi! I just wanted to make another FreeBSD qemu git head snaphot port update, and found both i386-softmmu and x86_64-softmmu no longer boot, they seem to hang early in the bios before it prints anything, last tb seems to be this loop: IN: 0x000f1b8e: mov0xf81a0,%ecx 0x000f1b94: cmp%ecx,%eax 0x000f1b96: jne0xf1b8e OUT: [size=184] 0x4000e440: mov$0xf81a0,%ebp 0x4000e445: mov%rbp,%rsi 0x4000e448: mov%rbp,%rdi 0x4000e44b: shr$0x7,%rsi 0x4000e44f: and$0xf003,%rdi 0x4000e456: and$0x1fe0,%esi 0x4000e45c: lea0x4f8(%rsi,%r14,1),%rsi 0x4000e464: cmp(%rsi),%rdi 0x4000e467: mov%rbp,%rdi 0x4000e46a: je 0x4000e477 0x4000e46c: xor%esi,%esi 0x4000e46e: callq 0x51fd30 0x4000e473: mov%eax,%ebp 0x4000e475: jmp0x4000e47d 0x4000e477: add0x18(%rsi),%rdi 0x4000e47b: mov(%rdi),%ebp 0x4000e47d: mov%ebp,%ebp 0x4000e47f: mov%rbp,%rbx 0x4000e482: mov(%r14),%r12 0x4000e485: mov%rbx,%r13 0x4000e488: sub%rbx,%r12 0x4000e48b: mov%r12,%rbx 0x4000e48e: mov%ebx,%ebx 0x4000e490: mov$0x10,%r15d 0x4000e496: mov%r15d,0xa0(%r14) 0x4000e49d: mov%r13,0x90(%r14) 0x4000e4a4: mov%r12,0x98(%r14) 0x4000e4ab: mov%rbp,0x8(%r14) 0x4000e4af: test %rbx,%rbx 0x4000e4b2: jne0x4000e4d8 0x4000e4b8: jmpq 0x4000e4bd 0x4000e4bd: mov$0xf1b98,%ebp 0x4000e4c2: mov%rbp,0x80(%r14) 0x4000e4c9: mov$0x802c05c80,%rax 0x4000e4d3: jmpq 0xb65b8e 0x4000e4d8: jmpq 0x4000e4dd 0x4000e4dd: mov$0xf1b8e,%ebp 0x4000e4e2: mov%rbp,0x80(%r14) 0x4000e4e9: mov$0x802c05c81,%rax 0x4000e4f3: jmpq 0xb65b8e Is 0xf81a0 an io port or how is it supposed to change? And, can anyone reproduce this on Linux? As I said this is without kvm... Thanx! :) Juergen
Re: [Qemu-devel] [patch 1/2] Pass QEMUIOWorker to qemu_notify_event
On 03/11/2010 08:45 PM, Marcelo Tosatti wrote:
This can be used later to introduce generic iothread workers.
Signed-off-by: Marcelo Tosatti
Could you rebase this? It failed to apply in a strange way that made me
nervous...
Regards,
Anthony Liguori
Index: qemu-ioworker/async.c
===
--- qemu-ioworker.orig/async.c
+++ qemu-ioworker/async.c
@@ -180,7 +180,7 @@ void qemu_bh_schedule(QEMUBH *bh)
bh->scheduled = 1;
bh->idle = 0;
/* stop the currently executing CPU to execute the BH ASAP */
-qemu_notify_event();
+qemu_notify_event(main_io_worker);
}
void qemu_bh_cancel(QEMUBH *bh)
Index: qemu-ioworker/hw/mac_dbdma.c
===
--- qemu-ioworker.orig/hw/mac_dbdma.c
+++ qemu-ioworker/hw/mac_dbdma.c
@@ -655,7 +655,7 @@ void DBDMA_register_channel(void *dbdma,
void DBDMA_schedule(void)
{
-qemu_notify_event();
+qemu_notify_event(main_io_worker);
}
static void
Index: qemu-ioworker/hw/virtio-net.c
===
--- qemu-ioworker.orig/hw/virtio-net.c
+++ qemu-ioworker/hw/virtio-net.c
@@ -359,7 +359,7 @@ static void virtio_net_handle_rx(VirtIOD
/* We now have RX buffers, signal to the IO thread to break out of the
* select to re-poll the tap file descriptor */
-qemu_notify_event();
+qemu_notify_event(main_io_worker);
}
static int virtio_net_can_receive(VLANClientState *nc)
Index: qemu-ioworker/qemu-common.h
===
--- qemu-ioworker.orig/qemu-common.h
+++ qemu-ioworker/qemu-common.h
@@ -234,11 +234,17 @@ typedef uint64_t pcibus_t;
void cpu_save(QEMUFile *f, void *opaque);
int cpu_load(QEMUFile *f, void *opaque, int version_id);
+typedef struct QEMUIOWorker {
+void *opaque;
+} QEMUIOWorker;
+
/* Force QEMU to stop what it's doing and service IO */
void qemu_service_io(void);
/* Force QEMU to process pending events */
-void qemu_notify_event(void);
+void qemu_notify_event(QEMUIOWorker *worker);
+
+extern QEMUIOWorker *main_io_worker;
/* Unblock cpu */
void qemu_cpu_kick(void *env);
Index: qemu-ioworker/vl.c
===
--- qemu-ioworker.orig/vl.c
+++ qemu-ioworker/vl.c
@@ -274,6 +274,9 @@ uint8_t qemu_uuid[16];
static QEMUBootSetHandler *boot_set_handler;
static void *boot_set_opaque;
+QEMUIOWorker iothread_worker;
+QEMUIOWorker *main_io_worker =&iothread_worker;
+
#ifdef SIGRTMIN
#define SIG_IPI (SIGRTMIN+4)
#else
@@ -885,7 +888,7 @@ void qemu_mod_timer(QEMUTimer *ts, int64
}
/* Interrupt execution to force deadline recalculation. */
if (use_icount)
-qemu_notify_event();
+qemu_notify_event(main_io_worker);
}
}
@@ -1062,7 +1065,7 @@ static void host_alarm_handler(int host_
}
#endif
timer_alarm_pending = 1;
-qemu_notify_event();
+qemu_notify_event(main_io_worker);
}
}
@@ -2928,7 +2931,7 @@ static int ram_load(QEMUFile *f, void *o
void qemu_service_io(void)
{
-qemu_notify_event();
+qemu_notify_event(main_io_worker);
}
/***/
@@ -3180,26 +3183,26 @@ void qemu_system_reset_request(void)
} else {
reset_requested = 1;
}
-qemu_notify_event();
+qemu_notify_event(main_io_worker);
}
void qemu_system_shutdown_request(void)
{
shutdown_requested = 1;
-qemu_notify_event();
+qemu_notify_event(main_io_worker);
}
void qemu_system_powerdown_request(void)
{
powerdown_requested = 1;
-qemu_notify_event();
+qemu_notify_event(main_io_worker);
}
#ifdef CONFIG_IOTHREAD
static void qemu_system_vmstop_request(int reason)
{
vmstop_requested = reason;
-qemu_notify_event();
+qemu_notify_event(main_io_worker);
}
#endif
@@ -3341,7 +3344,7 @@ void qemu_cpu_kick(void *env)
return;
}
-void qemu_notify_event(void)
+void qemu_notify_event(QEMUIOWorker *worker)
{
CPUState *env = cpu_single_env;
@@ -3727,7 +3730,7 @@ void qemu_init_vcpu(void *_env)
tcg_init_vcpu(env);
}
-void qemu_notify_event(void)
+void qemu_notify_event(QEMUIOWorker *worker)
{
qemu_event_increment();
}
Re: [Qemu-devel] [PATCHv6 08/11] vhost: vhost net support
On Mon, Mar 22, 2010 at 03:58:58PM -0500, Anthony Liguori wrote:
> On 03/17/2010 06:08 AM, Michael S. Tsirkin wrote:
>> This adds vhost net device support in qemu. Will be tied to tap device
>> and virtio by following patches. Raw backend is currently missing,
>> will be worked on/submitted separately.
>>
>> Signed-off-by: Michael S. Tsirkin
>> ---
>> Makefile.target |2 +
>> configure | 37 +++
>> hw/vhost.c | 711
>> +++
>> hw/vhost.h | 48
>> hw/vhost_net.c | 195 +++
>> hw/vhost_net.h | 19 ++
>> 6 files changed, 1012 insertions(+), 0 deletions(-)
>> create mode 100644 hw/vhost.c
>> create mode 100644 hw/vhost.h
>> create mode 100644 hw/vhost_net.c
>> create mode 100644 hw/vhost_net.h
>>
>> diff --git a/Makefile.target b/Makefile.target
>> index 004a703..ea5207c 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -176,6 +176,8 @@ obj-y = vl.o async.o monitor.o pci.o pci_host.o
>> pcie_host.o machine.o gdbstub.o
>> # need to fix this properly
>> obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-pci.o
>> virtio-serial-bus.o
>> obj-y += event_notifier.o
>> +obj-y += vhost_net.o
>> +obj-$(CONFIG_VHOST_NET) += vhost.o
>> obj-y += rwhandler.o
>> obj-$(CONFIG_KVM) += kvm.o kvm-all.o
>> obj-$(CONFIG_ISA_MMIO) += isa_mmio.o
>> diff --git a/configure b/configure
>> index d728799..93015e3 100755
>> --- a/configure
>> +++ b/configure
>> @@ -263,6 +263,7 @@ vnc_tls=""
>> vnc_sasl=""
>> xen=""
>> linux_aio=""
>> +vhost_net=""
>>
>> gprof="no"
>> debug_tcg="no"
>> @@ -651,6 +652,10 @@ for opt do
>> ;;
>> --enable-docs) docs="yes"
>> ;;
>> + --disable-vhost-net) vhost_net="no"
>> + ;;
>> + --enable-vhost-net) vhost_net="yes"
>> + ;;
>> *) echo "ERROR: unknown option $opt"; show_help="yes"
>> ;;
>> esac
>> @@ -806,6 +811,8 @@ echo " --disable-blobs disable installing
>> provided firmware blobs"
>> echo " --kerneldir=PATH look for kernel includes in PATH"
>> echo " --enable-docsenable documentation build"
>> echo " --disable-docs disable documentation build"
>> +echo " --disable-vhost-net disable vhost-net acceleration support"
>> +echo " --enable-vhost-net enable vhost-net acceleration support"
>> echo ""
>> echo "NOTE: The object files are built at the place where configure is
>> launched"
>> exit 1
>> @@ -1498,6 +1505,32 @@ EOF
>> fi
>>
>> ##
>> +# test for vhost net
>> +
>> +if test "$vhost_net" != "no"; then
>> +if test "$kvm" != "no"; then
>> +cat> $TMPC<> +#include
>> +int main(void) { return 0; }
>> +EOF
>> +if compile_prog "$kvm_cflags" "" ; then
>> +vhost_net=yes
>> +else
>> +if "$vhost_net" == "yes" ; then
>> +feature_not_found "vhost-net"
>> +fi
>> +vhost_net=no
>> +fi
>> +else
>> +if "$vhost_net" == "yes" ; then
>> +echo -e "NOTE: vhost-net feature requires KVM
>> (--enable-kvm)."
>> +feature_not_found "vhost-net"
>> +fi
>>
>
> The indent is quite a bit off here but more importantly, if "$vhost_net"
> == "yes" is not a valid shell command. Instead, it ought to be:
>
> if test "$vhost_net" = "yes" ; then
>
> And likewise for the earlier check. I'll fold this fix into your series
> unless you'd like to respin for some reason.
So you fixed this yourself? Sure, thanks a lot!
> Regards,
>
> Anthony Liguori+ cpu_physical_memory_set_dirty(addr + bit * VHOST_LOG_PAGE);
Cool new sig.
--
MST
Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups
> On 03/22/2010 11:16 AM, Paul Brook wrote: > >> But look at the lguest virtio implement. We would definitely model a > >> VirtIOBus if we implemented something like that in qemu. VirtIO really > >> is designed to be a bus. > > > > When you say "bus" you actually mean point-point connection, right[1]? > > I don't see anything in virtio that allows arbitration of multiple > > devices, or any particular need for one as it can be handled by the host > > bus bindings. > > Virtio itself doesn't define any type of bus operations but is designed > to let it nicely fit into existing bus infrastructures. My understanding is that virtio specifies transport agnostic queueing API for passing buffers of data, and a small device specific config space. IMO a qemu virtio bus should implement exactly that. The host bridge submits buffers, and the device processes them. Allowing more than one device on this bus just adds complication for no benefit. The only reason to have multiple devices on a single bus is so that the can arbitrate a shared resource, and in this case we have no resources to share. If a single host device wants to support multiple virtio devices then it can create multiple busses. The purpose of the virtio bus is to expose the isolation between virtio device and host bridge/binding to the user. This allows virtio devices to be configured consistently without having to duplicate N hosts * M devices. > If you look at something like lguest, instead of piggying backing on > another bus, it introduces a bus as part of it's virtio infrastructure. > It's basically a shared memory page in a well known location. > > Anyway, if you were to implement virtio-lguest in qemu, it would have to > be a bus. Likewise, the virtio-s390 implement would also have to be a bus. I'm not convinced. AFAIKS the s390-virtio bus is just like any other peripheral bus. The only difference is that s390-virtio is something we invented, whereas PCI matches the programming interface used by real hardware. You may have a single cpu to s390-virtio bridge, and multiple s390-virtio to virtio bridges, but you still only need a single virtio device attached to each of those. By the same token, a virtio bus is not/need not be hot-pluggable. The (PCI/s390/whatever)-to-virtio bridge may be hot pluggable. This will cause virtio busses to appear and be destroyed. However this can happen atomically, so we will never need to add/remove a virtio device from an active bus. Paul
Re: [Qemu-devel] [PATCHv6 08/11] vhost: vhost net support
On 03/17/2010 06:08 AM, Michael S. Tsirkin wrote:
This adds vhost net device support in qemu. Will be tied to tap device
and virtio by following patches. Raw backend is currently missing,
will be worked on/submitted separately.
Signed-off-by: Michael S. Tsirkin
---
Makefile.target |2 +
configure | 37 +++
hw/vhost.c | 711 +++
hw/vhost.h | 48
hw/vhost_net.c | 195 +++
hw/vhost_net.h | 19 ++
6 files changed, 1012 insertions(+), 0 deletions(-)
create mode 100644 hw/vhost.c
create mode 100644 hw/vhost.h
create mode 100644 hw/vhost_net.c
create mode 100644 hw/vhost_net.h
diff --git a/Makefile.target b/Makefile.target
index 004a703..ea5207c 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -176,6 +176,8 @@ obj-y = vl.o async.o monitor.o pci.o pci_host.o pcie_host.o
machine.o gdbstub.o
# need to fix this properly
obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-pci.o
virtio-serial-bus.o
obj-y += event_notifier.o
+obj-y += vhost_net.o
+obj-$(CONFIG_VHOST_NET) += vhost.o
obj-y += rwhandler.o
obj-$(CONFIG_KVM) += kvm.o kvm-all.o
obj-$(CONFIG_ISA_MMIO) += isa_mmio.o
diff --git a/configure b/configure
index d728799..93015e3 100755
--- a/configure
+++ b/configure
@@ -263,6 +263,7 @@ vnc_tls=""
vnc_sasl=""
xen=""
linux_aio=""
+vhost_net=""
gprof="no"
debug_tcg="no"
@@ -651,6 +652,10 @@ for opt do
;;
--enable-docs) docs="yes"
;;
+ --disable-vhost-net) vhost_net="no"
+ ;;
+ --enable-vhost-net) vhost_net="yes"
+ ;;
*) echo "ERROR: unknown option $opt"; show_help="yes"
;;
esac
@@ -806,6 +811,8 @@ echo " --disable-blobs disable installing provided
firmware blobs"
echo " --kerneldir=PATH look for kernel includes in PATH"
echo " --enable-docsenable documentation build"
echo " --disable-docs disable documentation build"
+echo " --disable-vhost-net disable vhost-net acceleration support"
+echo " --enable-vhost-net enable vhost-net acceleration support"
echo ""
echo "NOTE: The object files are built at the place where configure is
launched"
exit 1
@@ -1498,6 +1505,32 @@ EOF
fi
##
+# test for vhost net
+
+if test "$vhost_net" != "no"; then
+if test "$kvm" != "no"; then
+cat> $TMPC<
+int main(void) { return 0; }
+EOF
+if compile_prog "$kvm_cflags" "" ; then
+vhost_net=yes
+else
+if "$vhost_net" == "yes" ; then
+feature_not_found "vhost-net"
+fi
+vhost_net=no
+fi
+else
+if "$vhost_net" == "yes" ; then
+echo -e "NOTE: vhost-net feature requires KVM (--enable-kvm)."
+feature_not_found "vhost-net"
+fi
The indent is quite a bit off here but more importantly, if "$vhost_net"
== "yes" is not a valid shell command. Instead, it ought to be:
if test "$vhost_net" = "yes" ; then
And likewise for the earlier check. I'll fold this fix into your series
unless you'd like to respin for some reason.
Regards,
Anthony Liguori+ cpu_physical_memory_set_dirty(addr + bit * VHOST_LOG_PAGE);
Re: [Qemu-devel] Supporting hypervisor specific APIs in libvirt
On Mon, Mar 22, 2010 at 02:25:00PM -0500, Anthony Liguori wrote: > Hi, > > I've mentioned this to a few folks already but I wanted to start a > proper thread. > > We're struggling in qemu with usability and one area that concerns me is > the disparity in features that are supported by qemu vs what's > implemented in libvirt. > > This isn't necessarily libvirt's problem if it's mission is to provide a > common hypervisor API that covers the most commonly used features. > > However, for qemu, we need an API that covers all of our features that > people can develop against. The ultimate question we need to figure out > is, should we encourage our users to always use libvirt or should we > build our own API for people (and libvirt) to consume. > > I don't think it's necessarily a big technical challenge for libvirt to > support qemu more completely. I think it amounts to introducing a > series of virQemu APIs that implement qemu specific functions. Over > time, qemu specific APIs can be deprecated in favour of more generic > virDomain APIs. The second thing I forgot to mention in my previous reply, is that we also need to get a much better idea of just which QEMU features missing in libvirt. Even if we provide a generic mechansim for setting QEMU specific features, we still need visibility into what needs to be added to the main generic libvirt XML / API format. I think the qdev & QMP work will be very useful in this respect, since both are pretty much self-describing. All that is missing from QEMU is a way to query/extra the qdev/QMP metadata from the QEMU binary in an automated fashion. eg, '-device qdev' can list all devices, but following on from that we need to query the properties known against each device type. I think it'd also be desirable to have a machine readable '-help' output format, so we can more reliably detect what args are available, since even with qdev + -device, we're still adding more command line args to QEMU over time like -netdev, -blockdev, etc. If we can easily get these details of qdev properties, QMP commands/args and ARGV from QEMU, then we can reliably identify just what is missing from libvirt & use that to guide development of generic APIs for the missing features. Regards, Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
[Qemu-devel] Re: [libvirt] Supporting hypervisor specific APIs in libvirt
On Mon, Mar 22, 2010 at 02:25:00PM -0500, Anthony Liguori wrote: > Hi, > > I've mentioned this to a few folks already but I wanted to start a > proper thread. > > We're struggling in qemu with usability and one area that concerns me is > the disparity in features that are supported by qemu vs what's > implemented in libvirt. > > This isn't necessarily libvirt's problem if it's mission is to provide a > common hypervisor API that covers the most commonly used features. That is more or less our current mission. If this mission leads to QEMU creating a non-libvirt based API & telling people to use that instead, then I'd say libvirt's mission needs to change to avoid that scenario ! I strongly believe that libvirt's strategy is good for application developers over the medium to long term. We need to figure out how to get rid of the short term pain from the feature timelag, rather than inventing a new library API for them to use. > However, for qemu, we need an API that covers all of our features that > people can develop against. The ultimate question we need to figure out > is, should we encourage our users to always use libvirt or should we > build our own API for people (and libvirt) to consume. > > I don't think it's necessarily a big technical challenge for libvirt to > support qemu more completely. I think it amounts to introducing a > series of virQemu APIs that implement qemu specific functions. Over > time, qemu specific APIs can be deprecated in favour of more generic > virDomain APIs. Stepping back a bit first, there are the two core areas in which people can be limited by libvirt currently. 1. Monitor commands 2. Command line flags Ultimately, IIUC, you are suggesting we need to allow arbitrary passthrough for both of these in libvirt. At the libvirt level, we have 3 core requirements 1. The XML format is extend only (new elements allowed, or add attributes or children to existing elements) 2. The C library API is append only (new symbols only) 3. The RPC wire protocol is append only (maps 1-1 to the C API generally) The core question for us as libvirt developers is how we could support QEMU specific features that may change arbitrarily, without it impacting on our ability to maintain these 3 requirements for the non-hypervisor specific APIs. We don't ever want to be in a situation where a QEMU specific API will require us to change the soname of the main libvirt library, or introduce incompatible wire protocol changes. If we were to introduce QEMU specific APIs, we also need a way to easily remove those over time, as & when we have them available as generic APIs. At the C API level, this to me suggests that we'd want to introduce a separate libvirt-qemu.so library for the QEMU specific APIs. This library would not have the same requirements of fixed long term ABI that the main libvirt.so did. We'd add QEMU APIs to libvirt-qemu.so any time needed, but remove them when the equivalent functionality were in libvirt.so, and increment the soname of libvirt-qemu.so at that point. At the wire protocol level, the protocol allows us to support multiple versioned protocols in parallel over the same data stream. So again there, we could define a sub-protocol for QEMU specific features for which we don't provide the indefinite ABI compatability. Finally the XML format is "easy" - just have a versioned XML namespace for extra pieces, that's distinct from the default namespace, again without the permanent long term compatability guarentees. There are, however, some bits that are unlikely to work when QEMU is under libvirt. Specifically any of the device backends that use stdio (eg, -serial stdio, or the ncurses graphics), simply because all libvirt spawned VMs are fully daemonized & so stdio is /dev/null Other items are hard, but not entirely impossible to solve. eg, any use of the 'script=' arg for -net devices doesn't work, because libvirt clears all capabilities from the QEMU process so it'll be lacking CAP_NET_ADMIN which most TAP device setup scripts in fact need. Some parts of the C library/wire protocol here are related to another feature I'd like to introduce for libvirt, namely a administrative library. eg a API to configure and manage the libvirtd daemon itself on the fly. This could easily hook into the wire protocol, but live as a separate libvirt-daemon.so library API in similar way to what I suggest for QEMU specific API > What's the feeling about this from the libvirt side of things? Is there > interest in support hypervisor specific interfaces should we be looking > to provide our own management interface for libvirt to consume? Adding yet another library in the stack isn't really going to solve the problem from the POV of libvirt users, but rather fork the community effort which I imagine we'd all rather avoid. Having to tell people to switch to a different library API to get access to a specific feature is a short term win, but
Re: [Qemu-devel] [PATCH 03/16] Convert io handlers to QLIST
On 03/11/2010 10:55 AM, Juan Quintela wrote:
Signed-off-by: Juan Quintela
Applied 3-7, thanks.
Regards,
Anthony Liguori
---
vl.c | 35 ++-
1 files changed, 14 insertions(+), 21 deletions(-)
diff --git a/vl.c b/vl.c
index 10d8e34..051eb1c 100644
--- a/vl.c
+++ b/vl.c
@@ -2597,10 +2597,12 @@ typedef struct IOHandlerRecord {
void *opaque;
/* temporary data */
struct pollfd *ufd;
-struct IOHandlerRecord *next;
+QLIST_ENTRY(IOHandlerRecord) next;
} IOHandlerRecord;
-static IOHandlerRecord *first_io_handler;
+static QLIST_HEAD(, IOHandlerRecord) io_handlers =
+QLIST_HEAD_INITIALIZER(io_handlers);
+
/* XXX: fd_read_poll should be suppressed, but an API change is
necessary in the character devices to suppress fd_can_read(). */
@@ -2610,28 +2612,22 @@ int qemu_set_fd_handler2(int fd,
IOHandler *fd_write,
void *opaque)
{
-IOHandlerRecord **pioh, *ioh;
+IOHandlerRecord *ioh;
if (!fd_read&& !fd_write) {
-pioh =&first_io_handler;
-for(;;) {
-ioh = *pioh;
-if (ioh == NULL)
-break;
+QLIST_FOREACH(ioh,&io_handlers, next) {
if (ioh->fd == fd) {
ioh->deleted = 1;
break;
}
-pioh =&ioh->next;
}
} else {
-for(ioh = first_io_handler; ioh != NULL; ioh = ioh->next) {
+QLIST_FOREACH(ioh,&io_handlers, next) {
if (ioh->fd == fd)
goto found;
}
ioh = qemu_mallocz(sizeof(IOHandlerRecord));
-ioh->next = first_io_handler;
-first_io_handler = ioh;
+QLIST_INSERT_HEAD(&io_handlers, ioh, next);
found:
ioh->fd = fd;
ioh->fd_read_poll = fd_read_poll;
@@ -3822,7 +3818,7 @@ void main_loop_wait(int timeout)
FD_ZERO(&rfds);
FD_ZERO(&wfds);
FD_ZERO(&xfds);
-for(ioh = first_io_handler; ioh != NULL; ioh = ioh->next) {
+QLIST_FOREACH(ioh,&io_handlers, next) {
if (ioh->deleted)
continue;
if (ioh->fd_read&&
@@ -3848,9 +3844,9 @@ void main_loop_wait(int timeout)
ret = select(nfds + 1,&rfds,&wfds,&xfds,&tv);
qemu_mutex_lock_iothread();
if (ret> 0) {
-IOHandlerRecord **pioh;
+IOHandlerRecord *pioh;
-for(ioh = first_io_handler; ioh != NULL; ioh = ioh->next) {
+QLIST_FOREACH(ioh,&io_handlers, next) {
if (!ioh->deleted&& ioh->fd_read&& FD_ISSET(ioh->fd,&rfds)) {
ioh->fd_read(ioh->opaque);
}
@@ -3860,14 +3856,11 @@ void main_loop_wait(int timeout)
}
/* remove deleted IO handlers */
- pioh =&first_io_handler;
- while (*pioh) {
-ioh = *pioh;
+QLIST_FOREACH_SAFE(ioh,&io_handlers, next, pioh) {
if (ioh->deleted) {
-*pioh = ioh->next;
+QLIST_REMOVE(ioh, next);
qemu_free(ioh);
-} else
-pioh =&ioh->next;
+}
}
}
Re: [Qemu-devel] [PATCH] CODING_STYLE: Reserve qemu_ prefix for library wrappers
On 03/11/2010 08:48 AM, Avi Kivity wrote: Signed-off-by: Avi Kivity --- CODING_STYLE |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/CODING_STYLE b/CODING_STYLE index a579cb1..92036f3 100644 --- a/CODING_STYLE +++ b/CODING_STYLE @@ -49,6 +49,9 @@ and is therefore likely to be changed. Typedefs are used to eliminate the redundant 'struct' keyword. It is the QEMU coding style. +When wrapping standard library functions, use the prefix qemu_ to alert +readers that they are seeing a wrapped version; otherwise avoid this prefix. + 4. Block structure Every indented statement is braced; even if the block contains just one Applied. Thanks. Regards, Anthony Liguori
[Qemu-devel] Supporting hypervisor specific APIs in libvirt
Hi, I've mentioned this to a few folks already but I wanted to start a proper thread. We're struggling in qemu with usability and one area that concerns me is the disparity in features that are supported by qemu vs what's implemented in libvirt. This isn't necessarily libvirt's problem if it's mission is to provide a common hypervisor API that covers the most commonly used features. However, for qemu, we need an API that covers all of our features that people can develop against. The ultimate question we need to figure out is, should we encourage our users to always use libvirt or should we build our own API for people (and libvirt) to consume. I don't think it's necessarily a big technical challenge for libvirt to support qemu more completely. I think it amounts to introducing a series of virQemu APIs that implement qemu specific functions. Over time, qemu specific APIs can be deprecated in favour of more generic virDomain APIs. What's the feeling about this from the libvirt side of things? Is there interest in support hypervisor specific interfaces should we be looking to provide our own management interface for libvirt to consume? Regards, Anthony Liguori
Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups
On 03/22/2010 11:16 AM, Paul Brook wrote: But look at the lguest virtio implement. We would definitely model a VirtIOBus if we implemented something like that in qemu. VirtIO really is designed to be a bus. When you say "bus" you actually mean point-point connection, right[1]? I don't see anything in virtio that allows arbitration of multiple devices, or any particular need for one as it can be handled by the host bus bindings. Virtio itself doesn't define any type of bus operations but is designed to let it nicely fit into existing bus infrastructures. If you look at something like lguest, instead of piggying backing on another bus, it introduces a bus as part of it's virtio infrastructure. It's basically a shared memory page in a well known location. Anyway, if you were to implement virtio-lguest in qemu, it would have to be a bus. Likewise, the virtio-s390 implement would also have to be a bus. So overall, virtio-pci is really just a special case of a virtio bus that only supports a single device. Whether you call that p2p I think is just a question of semantics. Regards, Anthony Liguori Paul [1] Technically I suppose a p-t-p connection is a degenerate case of a bus. While modern hardware busses (USB, PCIe) are electrically point-point, logically they are usually a shared bus topology.
[Qemu-devel] [PATCH] Fix bsd-user broken by commit b5ec5ce0e39d6e7ea707d5604a5f6d567dfd2f48
Signed-off-by: Juergen Lock
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -759,6 +759,10 @@ int main(int argc, char **argv)
}
cpu_model = NULL;
+#if defined(cpudef_setup)
+cpudef_setup(); /* parse cpu definitions in target config file (TBD) */
+#endif
+
optind = 1;
for(;;) {
if (optind >= argc)
Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups
On Mon, Mar 22, 2010 at 03:51:43PM +, Paul Brook wrote: > > > It's a classic OOP problem. > > > > > > VirtIOBlock is-a VirtIODevice, VirtIODevice is-a DeviceState > > > > > > VirtIOPCI is-a PCIDevice, PCIDevice is-a Device State. > > > > > > But VirtIODevice is-a VirtIOPCI device isn't always true so it can't be > > > an is-a relationship. Initially, this was true and that's why the code > > > was structured that way. Now that we have two type so of virtio > > > transports, we need to change the modelling. It needs to get inverted > > > into a has-a relationship. IOW, VirtIOPCI has-a VirtIODevice. > > > > > > When one device has-a one or more other devices, we model that as a Bus. > > > > Hmm. Is anything wrong with VirtIOPCIBlock which would be both a > > VirtIOBlock and VirtIOPCI device? > > You need to solve multiple inheritance with common (but divergent) ancestors. > > A single device may not have more than one DeviceState. i.e. the DeviceState > that is part of the VirtIOBlock must be the same as the DeviceState that is > part of the PCIDevice. However VirtIOBlock and PCIDevice can not know about > each other or about VirtIOPCIBlock. > > In the current code VirtIOPCIBlock already exists, though only for the > purposes of device creation/configuration. The remainder of the code and data > structures have clean separation between PCI and Virtio code. We can have VirtIOPCIBlock have a single DeviceState. > Anthony's suggestion is that we turn VirtIODevice into a user visible entity > (rather than an implementation detail). This makes the hoist bindings (virtio- > pci.c) completely independent of the virtio backends (e.g. virtio-block.c). > VirtIOPCIBlock ceases to exist, except maybe as a helpful alias to create a > VirtioPCI[Proxy]/VirtioBlock pair. > > > > It's just like SCSI. SCSIDisk is-a SCSIDevice, SCSIDevice is-a > > > DeviceState. > > > > > > LSIState is-a PCIDevice, PCIDevice is-a DeviceState. > > > > > > LSIState has-a SCSIDevice because LSIState implements the SCSIBus > > > interface. > > > > Yes but LSIState emulates a real HBI ... > > Not relevant. There should be nothing magic about virtio. If there is then > we're doing it wrong. > > Paul
[Qemu-devel] Re: How to create multiplexed chardevs?
Gerd Hoffmann wrote: > On 03/22/10 17:39, Jan Kiszka wrote: >> Hi Gerd, >> >> I think you mostly worked on this: >> >> How to specify -serial mon:stdio using the new syntax? I just ran into >> the problem having to define a serial port via -device isa-serial but >> wanting its terminal multiplexed with a monitor. -chardev mon:stdio is >> not understood. > > -chardev stdio,id=x,mux=on > -device isa-serial,chardev=x > -mon chardev=x > > Isn't limited to serial+monitor btw, you can mux everything with up to > four users. Almost perfect. I missed it as I was too lazy to look into the sources (aka: documentation missing :) ). And it starts the monitor enabled which should be fixed to keep it off until CTRL-A-C just like it used to be with "mon:". Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
[Qemu-devel] Re: How to create multiplexed chardevs?
On 03/22/10 17:39, Jan Kiszka wrote: Hi Gerd, I think you mostly worked on this: How to specify -serial mon:stdio using the new syntax? I just ran into the problem having to define a serial port via -device isa-serial but wanting its terminal multiplexed with a monitor. -chardev mon:stdio is not understood. -chardev stdio,id=x,mux=on -device isa-serial,chardev=x -mon chardev=x Isn't limited to serial+monitor btw, you can mux everything with up to four users. HTH, Gerd
Re: [Qemu-devel] virtio block device and sysfs
Jamie Lokier wrote: > john cooper wrote: >> All that said, I like the alternate choice of adding a >> special virtio request far better. It is actually simpler >> (and more maintainable IMO) than going through the >> gyrations of stuffing the S/N data through PCI config >> space. > > And it needn't have a 20 character limit. Technically yes it can be longer. Although IIRC the 20/21 byte assumption is fairly entrenched in related code. It may also be encouraged if the ATA interface is used to expose the data (under windows for instance). -john -- john.coo...@redhat.com
[Qemu-devel] How to create multiplexed chardevs?
Hi Gerd, I think you mostly worked on this: How to specify -serial mon:stdio using the new syntax? I just ran into the problem having to define a serial port via -device isa-serial but wanting its terminal multiplexed with a monitor. -chardev mon:stdio is not understood. BTW, there is are thousand ways of shooting yourself into the foot (up to crashing qemu) by accidentally combining old and new-style options. I think we should catch and forbid any mixture instead of generating error messages like this: qemu-system-x86_64 -mon monitor -chardev stdio,id=monitor -monitor vc tried to create id "monitor" twice for "chardev" parse error: vc Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] virtio block device and sysfs
john cooper wrote: > All that said, I like the alternate choice of adding a > special virtio request far better. It is actually simpler > (and more maintainable IMO) than going through the > gyrations of stuffing the S/N data through PCI config > space. And it needn't have a 20 character limit. -- Jamie
Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups
> But look at the lguest virtio implement. We would definitely model a > VirtIOBus if we implemented something like that in qemu. VirtIO really > is designed to be a bus. When you say "bus" you actually mean point-point connection, right[1]? I don't see anything in virtio that allows arbitration of multiple devices, or any particular need for one as it can be handled by the host bus bindings. Paul [1] Technically I suppose a p-t-p connection is a degenerate case of a bus. While modern hardware busses (USB, PCIe) are electrically point-point, logically they are usually a shared bus topology.
Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups
> > It's a classic OOP problem. > > > > VirtIOBlock is-a VirtIODevice, VirtIODevice is-a DeviceState > > > > VirtIOPCI is-a PCIDevice, PCIDevice is-a Device State. > > > > But VirtIODevice is-a VirtIOPCI device isn't always true so it can't be > > an is-a relationship. Initially, this was true and that's why the code > > was structured that way. Now that we have two type so of virtio > > transports, we need to change the modelling. It needs to get inverted > > into a has-a relationship. IOW, VirtIOPCI has-a VirtIODevice. > > > > When one device has-a one or more other devices, we model that as a Bus. > > Hmm. Is anything wrong with VirtIOPCIBlock which would be both a > VirtIOBlock and VirtIOPCI device? You need to solve multiple inheritance with common (but divergent) ancestors. A single device may not have more than one DeviceState. i.e. the DeviceState that is part of the VirtIOBlock must be the same as the DeviceState that is part of the PCIDevice. However VirtIOBlock and PCIDevice can not know about each other or about VirtIOPCIBlock. In the current code VirtIOPCIBlock already exists, though only for the purposes of device creation/configuration. The remainder of the code and data structures have clean separation between PCI and Virtio code. Anthony's suggestion is that we turn VirtIODevice into a user visible entity (rather than an implementation detail). This makes the hoist bindings (virtio- pci.c) completely independent of the virtio backends (e.g. virtio-block.c). VirtIOPCIBlock ceases to exist, except maybe as a helpful alias to create a VirtioPCI[Proxy]/VirtioBlock pair. > > It's just like SCSI. SCSIDisk is-a SCSIDevice, SCSIDevice is-a > > DeviceState. > > > > LSIState is-a PCIDevice, PCIDevice is-a DeviceState. > > > > LSIState has-a SCSIDevice because LSIState implements the SCSIBus > > interface. > > Yes but LSIState emulates a real HBI ... Not relevant. There should be nothing magic about virtio. If there is then we're doing it wrong. Paul
Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups
On 03/22/2010 10:17 AM, Michael S. Tsirkin wrote: On Mon, Mar 22, 2010 at 10:03:29AM -0500, Anthony Liguori wrote: On 03/22/2010 09:50 AM, Michael S. Tsirkin wrote: On Mon, Mar 22, 2010 at 09:49:03AM -0500, Anthony Liguori wrote: On 03/22/2010 08:30 AM, Paul Brook wrote: A VirtIOBlock device cannot be a VirtIODevice while being a VirtIOPCIProxy (proxy is a poor name, btw). It really ought to be: DeviceState ->VirtIODevice ->VirtIOBlock and: PCIDevice ->VirtIOPCI : implements VirtIOBus The interface between the VirtIODevice and VirtIOBus is the virtio transport. The main reason a separate bus is needed is the same reason it's needed in Linux. VirtIOBlock has to be tied to some bus. It cannot be tied to the PCI bus without having it be part of the implementation detail. Introducing another bus type fixes this (and it's what we do in the kernel). Why does virtio need a device state and bus at all? Because you need VirtIOBlock to have qdev properties that can be set. You also need VirtIOPCI to have separate qdev properties that can be set. Can't it just be an internal implementation interface, which happens to be used by all devices that happen to exposed a block device over a virtio transport? Theoretically, yes, but given the rest of the infrastructure's interaction with qdev, making it a device makes the most sense IMHO. Does this boil down to qdev wanting to be the 1st field in the structure, again? We can just lift that limitation. No, I don't think it's relevant at all. It's a classic OOP problem. VirtIOBlock is-a VirtIODevice, VirtIODevice is-a DeviceState VirtIOPCI is-a PCIDevice, PCIDevice is-a Device State. But VirtIODevice is-a VirtIOPCI device isn't always true so it can't be an is-a relationship. Initially, this was true and that's why the code was structured that way. Now that we have two type so of virtio transports, we need to change the modelling. It needs to get inverted into a has-a relationship. IOW, VirtIOPCI has-a VirtIODevice. When one device has-a one or more other devices, we model that as a Bus. Hmm. Is anything wrong with VirtIOPCIBlock which would be both a VirtIOBlock and VirtIOPCI device? The problem is, VirtIODevice needs to interact with VirtIOPCI. If you do this as: VirtIOBlock -> VirtIOPCIBlock VirtIOPCIDevice -> Then you need to redirect through the hierarchy. It gets messy pretty quickly. That's sort of what we do with VirtIOPCIProxy today and it's pretty ugly. It's just like SCSI. SCSIDisk is-a SCSIDevice, SCSIDevice is-a DeviceState. LSIState is-a PCIDevice, PCIDevice is-a DeviceState. LSIState has-a SCSIDevice because LSIState implements the SCSIBus interface. Yes but LSIState emulates a real HBI ... But look at the lguest virtio implement. We would definitely model a VirtIOBus if we implemented something like that in qemu. VirtIO really is designed to be a bus. I can't envision any reason why we would ever want to have two MSI vectors for a given queue. No. OTOH whether we want a shared vector or a per-vq vector might depend on the device being used. Yup. From a users perspective, we don't want them to create two separate devices and manipulate parameters. We definitely want one interface where we can manipulate both VirtIODevice and VirtIOPCI parameters. Regards, Anthony Liguori Will a bus really help? Where would we put the # of vectors? I think this can't be a virtio-pci bus property as it might be different between different virtio pci devices. There would be a nvectors property of virtio-pci, you'd create a virtio-pci device, set nvectors, and add a VirtIODevice to it. Sounds obtuse from a user's perspective so we'd want to simplify the syntax. But in terms of internal modelling, it really simplifies things tremendously. Regards, Anthony Liguori
Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups
On Mon, Mar 22, 2010 at 10:03:29AM -0500, Anthony Liguori wrote: > On 03/22/2010 09:50 AM, Michael S. Tsirkin wrote: >> On Mon, Mar 22, 2010 at 09:49:03AM -0500, Anthony Liguori wrote: >> >>> On 03/22/2010 08:30 AM, Paul Brook wrote: >>> > A VirtIOBlock device cannot be a VirtIODevice while being a > VirtIOPCIProxy (proxy is a poor name, btw). > > It really ought to be: > > DeviceState -> VirtIODevice -> VirtIOBlock > > and: > > PCIDevice -> VirtIOPCI : implements VirtIOBus > > The interface between the VirtIODevice and VirtIOBus is the virtio > transport. > > The main reason a separate bus is needed is the same reason it's needed > in Linux. VirtIOBlock has to be tied to some bus. It cannot be tied to > the PCI bus without having it be part of the implementation detail. > Introducing another bus type fixes this (and it's what we do in the >kernel). > > Why does virtio need a device state and bus at all? >>> Because you need VirtIOBlock to have qdev properties that can be set. >>> >>> You also need VirtIOPCI to have separate qdev properties that can be set. >>> >>> Can't it just be an internal implementation interface, which happens to be used by all devices that happen to exposed a block device over a virtio transport? >>> Theoretically, yes, but given the rest of the infrastructure's >>> interaction with qdev, making it a device makes the most sense IMHO. >>> >> Does this boil down to qdev wanting to be the 1st field >> in the structure, again? We can just lift that limitation. >> > > No, I don't think it's relevant at all. > > It's a classic OOP problem. > > VirtIOBlock is-a VirtIODevice, VirtIODevice is-a DeviceState > > VirtIOPCI is-a PCIDevice, PCIDevice is-a Device State. > > But VirtIODevice is-a VirtIOPCI device isn't always true so it can't be > an is-a relationship. Initially, this was true and that's why the code > was structured that way. Now that we have two type so of virtio > transports, we need to change the modelling. It needs to get inverted > into a has-a relationship. IOW, VirtIOPCI has-a VirtIODevice. > > When one device has-a one or more other devices, we model that as a Bus. Hmm. Is anything wrong with VirtIOPCIBlock which would be both a VirtIOBlock and VirtIOPCI device? > It's just like SCSI. SCSIDisk is-a SCSIDevice, SCSIDevice is-a DeviceState. > > LSIState is-a PCIDevice, PCIDevice is-a DeviceState. > > LSIState has-a SCSIDevice because LSIState implements the SCSIBus interface. Yes but LSIState emulates a real HBI ... >>> I can't envision any reason why we would ever want to have two MSI >>> vectors for a given queue. >>> >> No. OTOH whether we want a shared vector or a per-vq vector >> might depend on the device being used. >> > > Yup. From a users perspective, we don't want them to create two > separate devices and manipulate parameters. We definitely want one > interface where we can manipulate both VirtIODevice and VirtIOPCI > parameters. > > Regards, > > Anthony Liguori Will a bus really help? Where would we put the # of vectors? I think this can't be a virtio-pci bus property as it might be different between different virtio pci devices. -- MST
Re: [Qemu-devel] virtio block device and sysfs
> > John attempted this and it was reverted because the implementation > > exhausted the PCI config space. > > I don't understand that. Existig hardware devices dump much more of > their data such as vendor and model type information into the config > space, how can using a field that real hardware uses exhaust the > config space? I think you (collectively) are confusing three different things: - PCI config space: Not used in any significant way other than the standard fields (i.e. basic device ID and configuring BARs). Slow/hard to access. - PCI IO BAR: Used by virtio-pci devices to expose the virtio config space. Extremely limited resource (max. 64k for the whole system). On real hardware only really used for legacy interfaces because it's slow, cumbersome, and x86 specific. Unfortunately it currently seems to lower overhead than MMIO on KVM :-( - PCI MEM BAR (usually MMIO): How real devices expose themselves. Relatively large address space (megabytes) available. Paul
Re: [Qemu-devel] virtio block device and sysfs
Marc Haber wrote: Hi, On Mon, Mar 22, 2010 at 02:26:11AM -0400, john cooper wrote: This is a tad ironic as that is how this saga begun. Namely stuffing 20 bytes of serial number string into the virtio-blk PCI config space on qemu's side and pushing it over to the guest driver. I exposed this to the guest app via a new ioctl cmd which itself was the original point of contention. Someone took issue with introducing a new interface citing the existence of ATA and SCSI counterparts. However dragging in the associated baggage in order to emulate those interfaces unintentionally bloated usage of the config space to the point of breakage. To address this I'd moved from using config space to an unused BAR which (understandably) didn't go over too well. Somewhere along the line Rusty posted a minimal alternative version which directly used a virtio request to retrieve the data from qemu which is arguably the right way to do the job. *argh* That sounds like politics. Well, maybe. But it is hard to argue with anyone calling the ATA_IDENTIFY interface ugly -- it is. Concerning qemu->virtio_blk communication, I don't think an argument to use PCI config space exists after one looks at the implementation of adding a new virtio request. That said we still had a dispute over what interface would be used to pass the S/N back to the guest: a new interface or reuse of an existing interface (eg: ATA IDENTIFY). That's where things fizzled when we couldn't immediately resolve the issue. So publishing the S/N in /sys would seem to side step this snag. Re-using an existing interface would probable make it easier for non-Linux OSses to also take advantage of this, since their ATA driver is already there. Exactly my singular motivation and honestly the only redeeming quality of propagating that crusty interface. I could have swore I sent out a guest-driver-app-interface-less version of the patch using virtio to pass the S/N but didn't find it in the archives. I did however locate it and can bring it forward as a reference for the above if interest exists. If it brings the issue forward and gives me hope to be able to do what I want to do in a reasonable time frame, why not. Just time. But I'll resurrect the patch so we don't have to go through all of this again. -john -- john.coo...@third-harmonic.com
Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups
On 03/22/2010 09:50 AM, Michael S. Tsirkin wrote: On Mon, Mar 22, 2010 at 09:49:03AM -0500, Anthony Liguori wrote: On 03/22/2010 08:30 AM, Paul Brook wrote: A VirtIOBlock device cannot be a VirtIODevice while being a VirtIOPCIProxy (proxy is a poor name, btw). It really ought to be: DeviceState -> VirtIODevice -> VirtIOBlock and: PCIDevice -> VirtIOPCI : implements VirtIOBus The interface between the VirtIODevice and VirtIOBus is the virtio transport. The main reason a separate bus is needed is the same reason it's needed in Linux. VirtIOBlock has to be tied to some bus. It cannot be tied to the PCI bus without having it be part of the implementation detail. Introducing another bus type fixes this (and it's what we do in the kernel). Why does virtio need a device state and bus at all? Because you need VirtIOBlock to have qdev properties that can be set. You also need VirtIOPCI to have separate qdev properties that can be set. Can't it just be an internal implementation interface, which happens to be used by all devices that happen to exposed a block device over a virtio transport? Theoretically, yes, but given the rest of the infrastructure's interaction with qdev, making it a device makes the most sense IMHO. Does this boil down to qdev wanting to be the 1st field in the structure, again? We can just lift that limitation. No, I don't think it's relevant at all. It's a classic OOP problem. VirtIOBlock is-a VirtIODevice, VirtIODevice is-a DeviceState VirtIOPCI is-a PCIDevice, PCIDevice is-a Device State. But VirtIODevice is-a VirtIOPCI device isn't always true so it can't be an is-a relationship. Initially, this was true and that's why the code was structured that way. Now that we have two type so of virtio transports, we need to change the modelling. It needs to get inverted into a has-a relationship. IOW, VirtIOPCI has-a VirtIODevice. When one device has-a one or more other devices, we model that as a Bus. It's just like SCSI. SCSIDisk is-a SCSIDevice, SCSIDevice is-a DeviceState. LSIState is-a PCIDevice, PCIDevice is-a DeviceState. LSIState has-a SCSIDevice because LSIState implements the SCSIBus interface. I can't envision any reason why we would ever want to have two MSI vectors for a given queue. No. OTOH whether we want a shared vector or a per-vq vector might depend on the device being used. Yup. From a users perspective, we don't want them to create two separate devices and manipulate parameters. We definitely want one interface where we can manipulate both VirtIODevice and VirtIOPCI parameters. Regards, Anthony Liguori
Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups
On Mon, Mar 22, 2010 at 09:49:03AM -0500, Anthony Liguori wrote: > On 03/22/2010 08:30 AM, Paul Brook wrote: >>> A VirtIOBlock device cannot be a VirtIODevice while being a >>> VirtIOPCIProxy (proxy is a poor name, btw). >>> >>> It really ought to be: >>> >>> DeviceState -> VirtIODevice -> VirtIOBlock >>> >>> and: >>> >>> PCIDevice -> VirtIOPCI : implements VirtIOBus >>> >>> The interface between the VirtIODevice and VirtIOBus is the virtio >>> transport. >>> >>> The main reason a separate bus is needed is the same reason it's needed >>> in Linux. VirtIOBlock has to be tied to some bus. It cannot be tied to >>> the PCI bus without having it be part of the implementation detail. >>> Introducing another bus type fixes this (and it's what we do in the >>> kernel). >>> >> Why does virtio need a device state and bus at all? >> > > Because you need VirtIOBlock to have qdev properties that can be set. > > You also need VirtIOPCI to have separate qdev properties that can be set. > >> Can't it just be an internal implementation interface, which happens to be >> used by all devices that happen to exposed a block device over a virtio >> transport? >> > > Theoretically, yes, but given the rest of the infrastructure's > interaction with qdev, making it a device makes the most sense IMHO. Does this boil down to qdev wanting to be the 1st field in the structure, again? We can just lift that limitation. >> If you have a virtio bus then IMO the PCI bridge device should be independent >> of the virtio device that is connected to it. >> > > Yes, that's the point I'm making. IOW, there shouldn't be a > "virtio-net-pci" device. Instead, there should be a "virtio-pci" device > that implements a VirtIOBus and then we add a single VirtIODevice to it > like "virtio-net". > > For something like MSI vector support, virtio-net really should have no > knowledge of MSI-x. Instead, you should specific nvectors to virtio-pci > and then virtio-pci should decide how to tie individual queue > notifications to the amount of MSI vectors it has. > > I can't envision any reason why we would ever want to have two MSI > vectors for a given queue. No. OTOH whether we want a shared vector or a per-vq vector might depend on the device being used. > Regards, > > Anthony Liguori > >> Paul >>
[Qemu-devel] Re: virtio block device and sysfs
On 03/22/2010 09:46 AM, Michael S. Tsirkin wrote: On Tue, Mar 09, 2010 at 03:34:07PM -0600, Anthony Liguori wrote: John attempted this and it was reverted because the implementation exhausted the PCI config space. Not config space really, John's implementation put ATA_IDENTITY (512 bytes) in PCI IO memory, PCI spec limits each such BAR to 256 bytes. Yes, I should know better than to be sloppy with my words on this :-) I meant, it exhausted the virtio pci config space which is limited by the maximum size of PCI IO memory. In theory, virtio has no config space limitation but virtio pci does. Regards, Anthony Liguori
[Qemu-devel] Re: virtio block device and sysfs
On Tue, Mar 09, 2010 at 03:34:07PM -0600, Anthony Liguori wrote: > John attempted this and it was reverted because the implementation > exhausted the PCI config space. Not config space really, John's implementation put ATA_IDENTITY (512 bytes) in PCI IO memory, PCI spec limits each such BAR to 256 bytes. -- MST
Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups
On 03/22/2010 08:30 AM, Paul Brook wrote: A VirtIOBlock device cannot be a VirtIODevice while being a VirtIOPCIProxy (proxy is a poor name, btw). It really ought to be: DeviceState -> VirtIODevice -> VirtIOBlock and: PCIDevice -> VirtIOPCI : implements VirtIOBus The interface between the VirtIODevice and VirtIOBus is the virtio transport. The main reason a separate bus is needed is the same reason it's needed in Linux. VirtIOBlock has to be tied to some bus. It cannot be tied to the PCI bus without having it be part of the implementation detail. Introducing another bus type fixes this (and it's what we do in the kernel). Why does virtio need a device state and bus at all? Because you need VirtIOBlock to have qdev properties that can be set. You also need VirtIOPCI to have separate qdev properties that can be set. Can't it just be an internal implementation interface, which happens to be used by all devices that happen to exposed a block device over a virtio transport? Theoretically, yes, but given the rest of the infrastructure's interaction with qdev, making it a device makes the most sense IMHO. If you have a virtio bus then IMO the PCI bridge device should be independent of the virtio device that is connected to it. Yes, that's the point I'm making. IOW, there shouldn't be a "virtio-net-pci" device. Instead, there should be a "virtio-pci" device that implements a VirtIOBus and then we add a single VirtIODevice to it like "virtio-net". For something like MSI vector support, virtio-net really should have no knowledge of MSI-x. Instead, you should specific nvectors to virtio-pci and then virtio-pci should decide how to tie individual queue notifications to the amount of MSI vectors it has. I can't envision any reason why we would ever want to have two MSI vectors for a given queue. Regards, Anthony Liguori Paul
Re: [Qemu-devel] virtio block device and sysfs
Marc Haber wrote: > Hi, > > On Tue, Mar 09, 2010 at 03:34:07PM -0600, Anthony Liguori wrote: >> On 03/06/2010 04:42 PM, Marc Haber wrote: >>> My goal is to have a possibility to give a "speaking" name to any >>> block device handed into a guest instance by the host. That name >>> should be visible inside the guest, just as a LV is visible with its >>> name in the system running the LVM. >>> >>> For example I would like to say on the qemu or kvm command line >>> '-drive file=some-file,label=some-label,if=virtio', and have the >>> string "some-label" show up somewhere in /sys/block in the guest, much >>> as /sys/block/sda/device/model shows the hardware vendor and type for >>> a standard SATA disk. The guest could then handle the information >>> passed into it by the host with udev rules, allowing fstab constructs >>> like "mount /dev/virtio/block/by-label/some-label as /usr" >>> >> You probably would just want to plumb ,serial=X into the virtio-blk >> config space and have the driver use it. Then you can do >> /dev/block/by-id/X > > It the serial "number" can be alphanumeric, that would be great to have. It is simply a string of 20 characters, which AFAICT has its roots in the ATA S/N convention along with many other puzzling present day evils. >> John attempted this and it was reverted because the implementation >> exhausted the PCI config space. > > I don't understand that. Existig hardware devices dump much more of > their data such as vendor and model type information into the config > space, how can using a field that real hardware uses exhaust the > config space? If you are only consuming 20 bytes in the config space currently there shouldn't be a problem -- that's exactly where I had started with this work. The issue was rather of emulating an ATA_IDENTIFY interface where qemu was cobbling together the content of the user data to be ultimately passed to the guest's application. That data structure by convention consumes a legacy 512 byte disk sector which overflowed the expectation of a 256 byte limit on PCI config space. The rationale was to have qemu package up the data so the virtio driver could simply treat it as opaque. Note the motivation for reuse of the ATA_IDENTIFY interface was that of leveraging existing tools such as hdparm(8) (and its windows counterpart) which already speak that convention. So there was a reason we wound up in this unhappy place. All that said, I like the alternate choice of adding a special virtio request far better. It is actually simpler (and more maintainable IMO) than going through the gyrations of stuffing the S/N data through PCI config space. -john -- john.coo...@redhat.com
[Qemu-devel] Re: Build always fail on x86_32 host for i386_softmmu target
Jan Kiszka wrote: > Juan Quintela wrote: >> Jan Kiszka wrote: >>> Juan Quintela wrote: Bruce Majia wrote: > Hi, > > When I built qemu on my x86_32 host with following configure line: > try to do first a: make distclean > $ ./configure --prefix=/usr/local/qemus/master \ > --target-list=i386-softmmu > $ make there are several config.h & config.mak around (from old builds) you need to remove then 1st. >>> Doesn't the fact that this is required for a proper rebuild indicate >>> some issue in the build system? I just recall a recent multi-hour >>> debugging session of mine which turned out to be due to an inconsistent >>> build after moving forward in the git history (in qemu-kvm, but the code >>> change came from upstream). Or are there some fundamentally unresolvable >>> scenarios so that a distclean is mandatory after every git update? >> >> Humm, no, it should only happens if you update from a _very_ old qemu >> (very old means something between qemu-0.11 - 0.12). >> >> If you had need it after 0.12, it is a different bug and should be fixed. >> > > I think I tried to reproduced this, but I failed after purging the buggy g> output directory (out-of-tree build): > > http://thread.gmane.org/gmane.comp.emulators.kvm.devel/47499 > > Maybe you are luckier than I :), or we have to wait for the next issue > to come. Looking at it. I normally are "unlucky" person on that cases. It hasn't happened to me, but will take a look. Later, Juan.
[Qemu-devel] Re: Build always fail on x86_32 host for i386_softmmu target
Juan Quintela wrote: > Jan Kiszka wrote: >> Juan Quintela wrote: >>> Bruce Majia wrote: Hi, When I built qemu on my x86_32 host with following configure line: >>> try to do first a: >>> make distclean >>> $ ./configure --prefix=/usr/local/qemus/master \ --target-list=i386-softmmu $ make >>> there are several config.h & config.mak around (from old builds) you >>> need to remove then 1st. >> Doesn't the fact that this is required for a proper rebuild indicate >> some issue in the build system? I just recall a recent multi-hour >> debugging session of mine which turned out to be due to an inconsistent >> build after moving forward in the git history (in qemu-kvm, but the code >> change came from upstream). Or are there some fundamentally unresolvable >> scenarios so that a distclean is mandatory after every git update? > > Humm, no, it should only happens if you update from a _very_ old qemu > (very old means something between qemu-0.11 - 0.12). > > If you had need it after 0.12, it is a different bug and should be fixed. > I think I tried to reproduced this, but I failed after purging the buggy output directory (out-of-tree build): http://thread.gmane.org/gmane.comp.emulators.kvm.devel/47499 Maybe you are luckier than I :), or we have to wait for the next issue to come. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
[Qemu-devel] Re: Build always fail on x86_32 host for i386_softmmu target
Jan Kiszka wrote: > Juan Quintela wrote: >> Bruce Majia wrote: >>> Hi, >>> >>> When I built qemu on my x86_32 host with following configure line: >>> >> >> try to do first a: >> make distclean >> >>> $ ./configure --prefix=/usr/local/qemus/master \ >>> --target-list=i386-softmmu >>> $ make >> >> there are several config.h & config.mak around (from old builds) you >> need to remove then 1st. > > Doesn't the fact that this is required for a proper rebuild indicate > some issue in the build system? I just recall a recent multi-hour > debugging session of mine which turned out to be due to an inconsistent > build after moving forward in the git history (in qemu-kvm, but the code > change came from upstream). Or are there some fundamentally unresolvable > scenarios so that a distclean is mandatory after every git update? Humm, no, it should only happens if you update from a _very_ old qemu (very old means something between qemu-0.11 - 0.12). If you had need it after 0.12, it is a different bug and should be fixed. Later, Juan.
[Qemu-devel] Re: Build always fail on x86_32 host for i386_softmmu target
Juan Quintela wrote: > Bruce Majia wrote: >> Hi, >> >> When I built qemu on my x86_32 host with following configure line: >> > > try to do first a: > make distclean > >> $ ./configure --prefix=/usr/local/qemus/master \ >> --target-list=i386-softmmu >> $ make > > there are several config.h & config.mak around (from old builds) you > need to remove then 1st. Doesn't the fact that this is required for a proper rebuild indicate some issue in the build system? I just recall a recent multi-hour debugging session of mine which turned out to be due to an inconsistent build after moving forward in the git history (in qemu-kvm, but the code change came from upstream). Or are there some fundamentally unresolvable scenarios so that a distclean is mandatory after every git update? Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] Re: [PATCH 0/9] Virtio cleanups
> A VirtIOBlock device cannot be a VirtIODevice while being a > VirtIOPCIProxy (proxy is a poor name, btw). > > It really ought to be: > > DeviceState -> VirtIODevice -> VirtIOBlock > > and: > > PCIDevice -> VirtIOPCI : implements VirtIOBus > > The interface between the VirtIODevice and VirtIOBus is the virtio > transport. > > The main reason a separate bus is needed is the same reason it's needed > in Linux. VirtIOBlock has to be tied to some bus. It cannot be tied to > the PCI bus without having it be part of the implementation detail. > Introducing another bus type fixes this (and it's what we do in the > kernel). Why does virtio need a device state and bus at all? Can't it just be an internal implementation interface, which happens to be used by all devices that happen to exposed a block device over a virtio transport? If you have a virtio bus then IMO the PCI bridge device should be independent of the virtio device that is connected to it. Paul
Re: [Qemu-devel] virtio block device and sysfs
Hi, On Mon, Mar 22, 2010 at 02:26:11AM -0400, john cooper wrote: > This is a tad ironic as that is how this saga begun. Namely stuffing > 20 bytes of serial number string into the virtio-blk PCI config space > on qemu's side and pushing it over to the guest driver. I exposed this > to the guest app via a new ioctl cmd which itself was the original > point of contention. Someone took issue with introducing a new > interface citing the existence of ATA and SCSI counterparts. However > dragging in the associated baggage in order to emulate those interfaces > unintentionally bloated usage of the config space to the point of breakage. > To address this I'd moved from using config space to an unused BAR which > (understandably) didn't go over too well. Somewhere along the line Rusty > posted a minimal alternative version which directly used a virtio request > to retrieve the data from qemu which is arguably the right way to do the > job. *argh* That sounds like politics. > That said we still had a dispute over what interface would be used to > pass the S/N back to the guest: a new interface or reuse of an existing > interface (eg: ATA IDENTIFY). That's where things fizzled when we > couldn't immediately resolve the issue. So publishing the S/N in > /sys would seem to side step this snag. Re-using an existing interface would probable make it easier for non-Linux OSses to also take advantage of this, since their ATA driver is already there. > I could have swore I sent out a guest-driver-app-interface-less > version of the patch using virtio to pass the S/N but didn't find it in > the archives. I did however locate it and can bring it forward as a > reference for the above if interest exists. If it brings the issue forward and gives me hope to be able to do what I want to do in a reasonable time frame, why not. Does qemu have an issue tracker where a wishlist issue could be filed to have this tracked? Greetings Marc -- - Marc Haber | "I don't trust Computers. They | Mailadresse im Header Mannheim, Germany | lose things."Winona Ryder | Fon: *49 621 72739834 Nordisch by Nature | How to make an American Quilt | Fax: *49 3221 2323190
Re: [Qemu-devel] virtio block device and sysfs
Hi, On Tue, Mar 09, 2010 at 03:34:07PM -0600, Anthony Liguori wrote: > On 03/06/2010 04:42 PM, Marc Haber wrote: >> My goal is to have a possibility to give a "speaking" name to any >> block device handed into a guest instance by the host. That name >> should be visible inside the guest, just as a LV is visible with its >> name in the system running the LVM. >> >> For example I would like to say on the qemu or kvm command line >> '-drive file=some-file,label=some-label,if=virtio', and have the >> string "some-label" show up somewhere in /sys/block in the guest, much >> as /sys/block/sda/device/model shows the hardware vendor and type for >> a standard SATA disk. The guest could then handle the information >> passed into it by the host with udev rules, allowing fstab constructs >> like "mount /dev/virtio/block/by-label/some-label as /usr" >> > > You probably would just want to plumb ,serial=X into the virtio-blk > config space and have the driver use it. Then you can do > /dev/block/by-id/X It the serial "number" can be alphanumeric, that would be great to have. > John attempted this and it was reverted because the implementation > exhausted the PCI config space. I don't understand that. Existig hardware devices dump much more of their data such as vendor and model type information into the config space, how can using a field that real hardware uses exhaust the config space? Greetings Marc -- - Marc Haber | "I don't trust Computers. They | Mailadresse im Header Mannheim, Germany | lose things."Winona Ryder | Fon: *49 621 72739834 Nordisch by Nature | How to make an American Quilt | Fax: *49 3221 2323190
[Qemu-devel] [PATCH v2 1/2] qdev: Convert qdev_unplug() to QError
Note: our device unplug methods don't need conversion work, because
they can't currently fail.
Signed-off-by: Markus Armbruster
---
hw/qdev.c |3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index f45ed0f..c521115 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -287,8 +287,7 @@ int qdev_init(DeviceState *dev)
int qdev_unplug(DeviceState *dev)
{
if (!dev->parent_bus->allow_hotplug) {
-error_report("Bus %s does not support hotplugging",
- dev->parent_bus->name);
+qerror_report(QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
return -1;
}
assert(dev->info->unplug != NULL);
--
1.6.6.1
[Qemu-devel] [PATCH v2 2/2] monitor: convert do_device_del() to QObject, QError
Signed-off-by: Markus Armbruster
---
hw/qdev.c |8
hw/qdev.h |2 +-
qemu-monitor.hx |3 ++-
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index c521115..d3bf0fa 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -799,15 +799,15 @@ int do_device_add(Monitor *mon, const QDict *qdict,
QObject **ret_data)
return 0;
}
-void do_device_del(Monitor *mon, const QDict *qdict)
+int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
const char *id = qdict_get_str(qdict, "id");
DeviceState *dev;
dev = qdev_find_recursive(main_system_bus, id);
if (NULL == dev) {
-error_report("Device '%s' not found", id);
-return;
+qerror_report(QERR_DEVICE_NOT_FOUND, id);
+return -1;
}
-qdev_unplug(dev);
+return qdev_unplug(dev);
}
diff --git a/hw/qdev.h b/hw/qdev.h
index 9475705..40373c8 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -176,7 +176,7 @@ void qbus_free(BusState *bus);
void do_info_qtree(Monitor *mon);
void do_info_qdm(Monitor *mon);
int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
-void do_device_del(Monitor *mon, const QDict *qdict);
+int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
/*** qdev-properties.c ***/
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index ff5f099..31087bd 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -589,7 +589,8 @@ ETEXI
.args_type = "id:s",
.params = "device",
.help = "remove device",
-.mhandler.cmd = do_device_del,
+.user_print = monitor_user_noop,
+.mhandler.cmd_new = do_device_del,
},
STEXI
--
1.6.6.1
[Qemu-devel] [PATCH v2 0/2] monitor: convert do_device_del() to QObject, QError
v2: Supply a missing error conversion pointed out by Luiz Markus Armbruster (2): qdev: Convert qdev_unplug() to QError monitor: convert do_device_del() to QObject, QError hw/qdev.c | 11 +-- hw/qdev.h |2 +- qemu-monitor.hx |3 ++- 3 files changed, 8 insertions(+), 8 deletions(-)
[Qemu-devel] Re: [PATCH v2 00/11] monitor: New commands netdev_add, netdev_del
Note: This series depends on "[PATCH v2 0/6] error: Clean up after recent changes".
[Qemu-devel] [PATCH 6/7] blkdebug: Add events and rules
Block drivers can trigger a blkdebug event whenever they reach a place where it
could be useful to inject an error for testing/debugging purposes.
Rules are read from a blkdebug config file and describe which action is taken
when an event is triggered. For now this is only injecting an error (with a few
options) or changing the state (which is an integer). Rules can be declared to
be active only in a specific state; this way later rules can distiguish on
which path we came to trigger their event.
Signed-off-by: Kevin Wolf
---
block.c | 12 +++
block.h |9 ++
block/blkdebug.c | 249 +-
block_int.h |2 +
4 files changed, 271 insertions(+), 1 deletions(-)
diff --git a/block.c b/block.c
index e891544..122a620 100644
--- a/block.c
+++ b/block.c
@@ -1535,6 +1535,18 @@ int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
return drv->bdrv_load_vmstate(bs, buf, pos, size);
}
+void bdrv_debug_event(BlockDriverState *bs, BlkDebugEvent event)
+{
+BlockDriver *drv = bs->drv;
+
+if (!drv || !drv->bdrv_debug_event) {
+return;
+}
+
+return drv->bdrv_debug_event(bs, event);
+
+}
+
/**/
/* handling of snapshots */
diff --git a/block.h b/block.h
index edf5704..2fd8361 100644
--- a/block.h
+++ b/block.h
@@ -207,4 +207,13 @@ int bdrv_get_dirty(BlockDriverState *bs, int64_t sector);
void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector,
int nr_sectors);
int64_t bdrv_get_dirty_count(BlockDriverState *bs);
+
+
+typedef enum {
+BLKDBG_EVENT_MAX,
+} BlkDebugEvent;
+
+#define BLKDBG_EVENT(bs, evt) bdrv_debug_event(bs, evt)
+void bdrv_debug_event(BlockDriverState *bs, BlkDebugEvent event);
+
#endif
diff --git a/block/blkdebug.c b/block/blkdebug.c
index dad3ac6..0878a1b 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -46,6 +46,7 @@ typedef struct BlkdebugVars {
typedef struct BDRVBlkdebugState {
BlockDriverState *hd;
BlkdebugVars vars;
+QLIST_HEAD(list, BlkdebugRule) rules[BLKDBG_EVENT_MAX];
} BDRVBlkdebugState;
typedef struct BlkdebugAIOCB {
@@ -61,16 +62,210 @@ static AIOPool blkdebug_aio_pool = {
.cancel = blkdebug_aio_cancel,
};
+enum {
+ACTION_INJECT_ERROR,
+ACTION_SET_STATE,
+};
+
+typedef struct BlkdebugRule {
+BlkDebugEvent event;
+int action;
+int state;
+union {
+struct {
+int error;
+int immediately;
+int once;
+} inject;
+struct {
+int new_state;
+} set_state;
+} options;
+QLIST_ENTRY(BlkdebugRule) next;
+} BlkdebugRule;
+
+static QemuOptsList inject_error_opts = {
+.name = "inject-error",
+.head = QTAILQ_HEAD_INITIALIZER(inject_error_opts.head),
+.desc = {
+{
+.name = "event",
+.type = QEMU_OPT_STRING,
+},
+{
+.name = "state",
+.type = QEMU_OPT_NUMBER,
+},
+{
+.name = "errno",
+.type = QEMU_OPT_NUMBER,
+},
+{
+.name = "once",
+.type = QEMU_OPT_BOOL,
+},
+{
+.name = "immediately",
+.type = QEMU_OPT_BOOL,
+},
+{ /* end of list */ }
+},
+};
+
+static QemuOptsList set_state_opts = {
+.name = "set-state",
+.head = QTAILQ_HEAD_INITIALIZER(inject_error_opts.head),
+.desc = {
+{
+.name = "event",
+.type = QEMU_OPT_STRING,
+},
+{
+.name = "state",
+.type = QEMU_OPT_NUMBER,
+},
+{
+.name = "new_state",
+.type = QEMU_OPT_NUMBER,
+},
+{ /* end of list */ }
+},
+};
+
+static QemuOptsList *config_groups[] = {
+&inject_error_opts,
+&set_state_opts,
+NULL
+};
+
+static const char *event_names[BLKDBG_EVENT_MAX] = {
+};
+
+static int get_event_by_name(const char *name, BlkDebugEvent *event)
+{
+int i;
+
+for (i = 0; i < BLKDBG_EVENT_MAX; i++) {
+if (!strcmp(event_names[i], name)) {
+*event = i;
+return 0;
+}
+}
+
+return -1;
+}
+
+struct add_rule_data {
+BDRVBlkdebugState *s;
+int action;
+};
+
+static int add_rule(QemuOpts *opts, void *opaque)
+{
+struct add_rule_data *d = opaque;
+BDRVBlkdebugState *s = d->s;
+const char* event_name;
+BlkDebugEvent event;
+struct BlkdebugRule *rule;
+
+/* Find the right event for the rule */
+event_name = qemu_opt_get(opts, "event");
+if (!event_name || get_event_by_name(event_name, &event) < 0) {
+return -1;
+}
+
+/* Set attributes common for all actions */
+rule = qemu_mallocz(sizeof(*rule));
+*rule = (struct BlkdebugRule) {
+.event = event,
+.action = d->action,
+.state = qemu_opt_get_number(opts, "state", 0),
[Qemu-devel] [PATCH 7/7] qcow2: Trigger blkdebug events
This adds blkdebug events to qcow2 to allow injecting I/O errors in specific
places.
Signed-off-by: Kevin Wolf
---
block.h| 44
block/blkdebug.c | 42 ++
block/qcow2-cluster.c | 15 +++
block/qcow2-refcount.c | 18 ++
block/qcow2.c |6 ++
5 files changed, 125 insertions(+), 0 deletions(-)
diff --git a/block.h b/block.h
index 2fd8361..14443f1 100644
--- a/block.h
+++ b/block.h
@@ -210,6 +210,50 @@ int64_t bdrv_get_dirty_count(BlockDriverState *bs);
typedef enum {
+BLKDBG_L1_UPDATE,
+
+BLKDBG_L1_GROW_ALLOC_TABLE,
+BLKDBG_L1_GROW_WRITE_TABLE,
+BLKDBG_L1_GROW_ACTIVATE_TABLE,
+
+BLKDBG_L2_LOAD,
+BLKDBG_L2_UPDATE,
+BLKDBG_L2_UPDATE_COMPRESSED,
+BLKDBG_L2_ALLOC_COW_READ,
+BLKDBG_L2_ALLOC_WRITE,
+
+BLKDBG_READ,
+BLKDBG_READ_AIO,
+BLKDBG_READ_BACKING,
+BLKDBG_READ_BACKING_AIO,
+BLKDBG_READ_COMPRESSED,
+
+BLKDBG_WRITE_AIO,
+BLKDBG_WRITE_COMPRESSED,
+
+BLKDBG_VMSTATE_LOAD,
+BLKDBG_VMSTATE_SAVE,
+
+BLKDBG_COW_READ,
+BLKDBG_COW_WRITE,
+
+BLKDBG_REFTABLE_LOAD,
+BLKDBG_REFTABLE_GROW,
+
+BLKDBG_REFBLOCK_LOAD,
+BLKDBG_REFBLOCK_UPDATE,
+BLKDBG_REFBLOCK_UPDATE_PART,
+BLKDBG_REFBLOCK_ALLOC,
+BLKDBG_REFBLOCK_ALLOC_HOOKUP,
+BLKDBG_REFBLOCK_ALLOC_WRITE,
+BLKDBG_REFBLOCK_ALLOC_WRITE_BLOCKS,
+BLKDBG_REFBLOCK_ALLOC_WRITE_TABLE,
+BLKDBG_REFBLOCK_ALLOC_SWITCH_TABLE,
+
+BLKDBG_CLUSTER_ALLOC,
+BLKDBG_CLUSTER_ALLOC_BYTES,
+BLKDBG_CLUSTER_FREE,
+
BLKDBG_EVENT_MAX,
} BlkDebugEvent;
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 0878a1b..4a0bfe3 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -139,6 +139,48 @@ static QemuOptsList *config_groups[] = {
};
static const char *event_names[BLKDBG_EVENT_MAX] = {
+[BLKDBG_L1_UPDATE] = "l1_update",
+[BLKDBG_L1_GROW_ALLOC_TABLE]= "l1_grow.alloc_table",
+[BLKDBG_L1_GROW_WRITE_TABLE]= "l1_grow.write_table",
+[BLKDBG_L1_GROW_ACTIVATE_TABLE] = "l1_grow.activate_table",
+
+[BLKDBG_L2_LOAD]= "l2_load",
+[BLKDBG_L2_UPDATE] = "l2_update",
+[BLKDBG_L2_UPDATE_COMPRESSED] = "l2_update_compressed",
+[BLKDBG_L2_ALLOC_COW_READ] = "l2_alloc.cow_read",
+[BLKDBG_L2_ALLOC_WRITE] = "l2_alloc.write",
+
+[BLKDBG_READ] = "read",
+[BLKDBG_READ_AIO] = "read_aio",
+[BLKDBG_READ_BACKING] = "read_backing",
+[BLKDBG_READ_BACKING_AIO] = "read_backing_aio",
+[BLKDBG_READ_COMPRESSED]= "read_compressed",
+
+[BLKDBG_WRITE_AIO] = "write_aio",
+[BLKDBG_WRITE_COMPRESSED] = "write_compressed",
+
+[BLKDBG_VMSTATE_LOAD] = "vmstate_load",
+[BLKDBG_VMSTATE_SAVE] = "vmstate_save",
+
+[BLKDBG_COW_READ] = "cow_read",
+[BLKDBG_COW_WRITE] = "cow_write",
+
+[BLKDBG_REFTABLE_LOAD] = "reftable_load",
+[BLKDBG_REFTABLE_GROW] = "reftable_grow",
+
+[BLKDBG_REFBLOCK_LOAD] = "refblock_load",
+[BLKDBG_REFBLOCK_UPDATE]= "refblock_update",
+[BLKDBG_REFBLOCK_UPDATE_PART] = "refblock_update_part",
+[BLKDBG_REFBLOCK_ALLOC] = "refblock_alloc",
+[BLKDBG_REFBLOCK_ALLOC_HOOKUP] = "refblock_alloc.hookup",
+[BLKDBG_REFBLOCK_ALLOC_WRITE] = "refblock_alloc.write",
+[BLKDBG_REFBLOCK_ALLOC_WRITE_BLOCKS]= "refblock_alloc.write_blocks",
+[BLKDBG_REFBLOCK_ALLOC_WRITE_TABLE] = "refblock_alloc.write_table",
+[BLKDBG_REFBLOCK_ALLOC_SWITCH_TABLE]= "refblock_alloc.switch_table",
+
+[BLKDBG_CLUSTER_ALLOC] = "cluster_alloc",
+[BLKDBG_CLUSTER_ALLOC_BYTES]= "cluster_alloc_bytes",
+[BLKDBG_CLUSTER_FREE] = "cluster_free",
};
static int get_event_by_name(const char *name, BlkDebugEvent *event)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index b13b693..9b3d686 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -54,12 +54,14 @@ int qcow2_grow_l1_table(BlockDriverState *bs, int min_size)
memcpy(new_l1_table, s->l1_table, s->l1_size * sizeof(uint64_t));
/* write new table (align to cluster) */
+BLKDBG_EVENT(s->hd, BLKDBG_L1_GROW_ALLOC_TABLE);
new_l1_table_offset = qcow2_alloc_clusters(bs, new_l1_size2);
if (new_l1_table_offset < 0) {
qemu_free(new_l1_table);
return new_l1_table_offset;
}
+BLKDBG_EVENT(s->hd, BLKDBG_L1_GROW_WRITE_TABLE);
for(i = 0; i < s->l1_size; i++)
new_l1_table[i] = cpu_to_be64(new_l1_table[i]);
[Qemu-devel] [PATCH 5/7] Make qemu-config available for tools
To be able to use config files for blkdebug, we need to make these functions
available in the tools. This involves moving two functions that can only be
built in the context of the emulator.
Signed-off-by: Kevin Wolf
---
Makefile.objs|4 ++--
hw/qdev-properties.c | 19 ++-
hw/qdev.h|1 -
qemu-config.c| 17 -
qemu-tool.c |4
5 files changed, 24 insertions(+), 21 deletions(-)
diff --git a/Makefile.objs b/Makefile.objs
index ece02d5..930408b 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -8,7 +8,7 @@ qobject-obj-y += qerror.o
# block-obj-y is code used by both qemu system emulation and qemu-img
block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o
-block-obj-y += nbd.o block.o aio.o aes.o osdep.o
+block-obj-y += nbd.o block.o aio.o aes.o osdep.o qemu-config.o
block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
@@ -76,7 +76,7 @@ common-obj-y += buffered_file.o migration.o migration-tcp.o
qemu-sockets.o
common-obj-y += qemu-char.o savevm.o #aio.o
common-obj-y += msmouse.o ps2.o
common-obj-y += qdev.o qdev-properties.o
-common-obj-y += qemu-config.o block-migration.o
+common-obj-y += block-migration.o
common-obj-$(CONFIG_BRLAPI) += baum.o
common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 92d6793..d344b80 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -660,7 +660,7 @@ void qdev_prop_set_defaults(DeviceState *dev, Property
*props)
static QTAILQ_HEAD(, GlobalProperty) global_props =
QTAILQ_HEAD_INITIALIZER(global_props);
-void qdev_prop_register_global(GlobalProperty *prop)
+static void qdev_prop_register_global(GlobalProperty *prop)
{
QTAILQ_INSERT_TAIL(&global_props, prop, next);
}
@@ -688,3 +688,20 @@ void qdev_prop_set_globals(DeviceState *dev)
}
}
}
+
+static int qdev_add_one_global(QemuOpts *opts, void *opaque)
+{
+GlobalProperty *g;
+
+g = qemu_mallocz(sizeof(*g));
+g->driver = qemu_opt_get(opts, "driver");
+g->property = qemu_opt_get(opts, "property");
+g->value= qemu_opt_get(opts, "value");
+qdev_prop_register_global(g);
+return 0;
+}
+
+void qemu_add_globals(void)
+{
+qemu_opts_foreach(&qemu_global_opts, qdev_add_one_global, NULL, 0);
+}
diff --git a/hw/qdev.h b/hw/qdev.h
index 9475705..3a8e801 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -273,7 +273,6 @@ void qdev_prop_set_macaddr(DeviceState *dev, const char
*name, uint8_t *value);
void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value);
void qdev_prop_set_defaults(DeviceState *dev, Property *props);
-void qdev_prop_register_global(GlobalProperty *prop);
void qdev_prop_register_global_list(GlobalProperty *props);
void qdev_prop_set_globals(DeviceState *dev);
diff --git a/qemu-config.c b/qemu-config.c
index dedf177..ad91133 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -378,23 +378,6 @@ int qemu_global_option(const char *str)
return 0;
}
-static int qemu_add_one_global(QemuOpts *opts, void *opaque)
-{
-GlobalProperty *g;
-
-g = qemu_mallocz(sizeof(*g));
-g->driver = qemu_opt_get(opts, "driver");
-g->property = qemu_opt_get(opts, "property");
-g->value= qemu_opt_get(opts, "value");
-qdev_prop_register_global(g);
-return 0;
-}
-
-void qemu_add_globals(void)
-{
-qemu_opts_foreach(&qemu_global_opts, qemu_add_one_global, NULL, 0);
-}
-
struct ConfigWriteData {
QemuOptsList *list;
FILE *fp;
diff --git a/qemu-tool.c b/qemu-tool.c
index 97ca949..619b7b1 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -128,6 +128,10 @@ void loc_restore(Location *loc)
{
}
+void loc_set_file(const char *fname, int lno)
+{
+}
+
void error_report(const char *fmt, ...)
{
va_list args;
--
1.6.6.1
[Qemu-devel] [PATCH 4/7] blkdebug: Inject errors
Add a mechanism to inject errors instead of passing requests on. With no
further patches applied, you can use it by setting inject_errno in gdb.
Signed-off-by: Kevin Wolf
---
block/blkdebug.c | 81 ++
1 files changed, 81 insertions(+), 0 deletions(-)
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 2c7e0dd..dad3ac6 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -26,10 +26,41 @@
#include "block_int.h"
#include "module.h"
+#include
+
+typedef struct BlkdebugVars {
+int state;
+
+/* If inject_errno != 0, an error is injected for requests */
+int inject_errno;
+
+/* Decides if all future requests fail (false) or only the next one and
+ * after the next request inject_errno is reset to 0 (true) */
+bool inject_once;
+
+/* Decides if aio_readv/writev fails right away (true) or returns an error
+ * return value only in the callback (false) */
+bool inject_immediately;
+} BlkdebugVars;
+
typedef struct BDRVBlkdebugState {
BlockDriverState *hd;
+BlkdebugVars vars;
} BDRVBlkdebugState;
+typedef struct BlkdebugAIOCB {
+BlockDriverAIOCB common;
+QEMUBH *bh;
+int ret;
+} BlkdebugAIOCB;
+
+static void blkdebug_aio_cancel(BlockDriverAIOCB *blockacb);
+
+static AIOPool blkdebug_aio_pool = {
+.aiocb_size = sizeof(BlkdebugAIOCB),
+.cancel = blkdebug_aio_cancel,
+};
+
static int blkdebug_open(BlockDriverState *bs, const char *filename, int flags)
{
BDRVBlkdebugState *s = bs->opaque;
@@ -42,11 +73,56 @@ static int blkdebug_open(BlockDriverState *bs, const char
*filename, int flags)
return bdrv_file_open(&s->hd, filename, flags);
}
+static void error_callback_bh(void *opaque)
+{
+struct BlkdebugAIOCB *acb = opaque;
+qemu_bh_delete(acb->bh);
+acb->common.cb(acb->common.opaque, acb->ret);
+qemu_aio_release(acb);
+}
+
+static void blkdebug_aio_cancel(BlockDriverAIOCB *blockacb)
+{
+BlkdebugAIOCB *acb = (BlkdebugAIOCB*) blockacb;
+qemu_aio_release(acb);
+}
+
+static BlockDriverAIOCB *inject_error(BlockDriverState *bs,
+BlockDriverCompletionFunc *cb, void *opaque)
+{
+BDRVBlkdebugState *s = bs->opaque;
+int error = s->vars.inject_errno;
+struct BlkdebugAIOCB *acb;
+QEMUBH *bh;
+
+if (s->vars.inject_once) {
+s->vars.inject_errno = 0;
+}
+
+if (s->vars.inject_immediately) {
+return NULL;
+}
+
+acb = qemu_aio_get(&blkdebug_aio_pool, bs, cb, opaque);
+acb->ret = -error;
+
+bh = qemu_bh_new(error_callback_bh, acb);
+acb->bh = bh;
+qemu_bh_schedule(bh);
+
+return (BlockDriverAIOCB*) acb;
+}
+
static BlockDriverAIOCB *blkdebug_aio_readv(BlockDriverState *bs,
int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
BlockDriverCompletionFunc *cb, void *opaque)
{
BDRVBlkdebugState *s = bs->opaque;
+
+if (s->vars.inject_errno) {
+return inject_error(bs, cb, opaque);
+}
+
BlockDriverAIOCB *acb =
bdrv_aio_readv(s->hd, sector_num, qiov, nb_sectors, cb, opaque);
return acb;
@@ -57,6 +133,11 @@ static BlockDriverAIOCB
*blkdebug_aio_writev(BlockDriverState *bs,
BlockDriverCompletionFunc *cb, void *opaque)
{
BDRVBlkdebugState *s = bs->opaque;
+
+if (s->vars.inject_errno) {
+return inject_error(bs, cb, opaque);
+}
+
BlockDriverAIOCB *acb =
bdrv_aio_writev(s->hd, sector_num, qiov, nb_sectors, cb, opaque);
return acb;
--
1.6.6.1
[Qemu-devel] [PATCH 1/7] qemu-config: qemu_read_config_file() reads the normal config file
Introduce a new function qemu_read_config_file which reads the VM configuration
from a config file. Unlike qemu_config_parse it doesn't take a open file but a
filename and reduces code duplication as a side effect.
Signed-off-by: Kevin Wolf
---
qemu-config.c | 15 +++
qemu-config.h |2 ++
vl.c | 38 +-
3 files changed, 30 insertions(+), 25 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index 150157c..57dc9a5 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -489,3 +489,18 @@ out:
loc_pop(&loc);
return res;
}
+
+int qemu_read_config_file(const char *filename)
+{
+FILE *f = fopen(filename, "r");
+if (f == NULL) {
+return -errno;
+}
+
+if (qemu_config_parse(f, filename) != 0) {
+return -EINVAL;
+}
+fclose(f);
+
+return 0;
+}
diff --git a/qemu-config.h b/qemu-config.h
index f217c58..07a7a27 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -19,4 +19,6 @@ void qemu_add_globals(void);
void qemu_config_write(FILE *fp);
int qemu_config_parse(FILE *fp, const char *fname);
+int qemu_read_config_file(const char *filename);
+
#endif /* QEMU_CONFIG_H */
diff --git a/vl.c b/vl.c
index 6948157..08294ff 100644
--- a/vl.c
+++ b/vl.c
@@ -3828,25 +3828,17 @@ int main(int argc, char **argv, char **envp)
}
if (defconfig) {
-const char *fname;
-FILE *fp;
+int ret;
-fname = CONFIG_QEMU_CONFDIR "/qemu.conf";
-fp = fopen(fname, "r");
-if (fp) {
-if (qemu_config_parse(fp, fname) != 0) {
-exit(1);
-}
-fclose(fp);
+ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf");
+if (ret == -EINVAL) {
+exit(1);
}
-fname = CONFIG_QEMU_CONFDIR "/target-" TARGET_ARCH ".conf";
-fp = fopen(fname, "r");
-if (fp) {
-if (qemu_config_parse(fp, fname) != 0) {
-exit(1);
-}
-fclose(fp);
+ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR
+"/target-" TARGET_ARCH ".conf");
+if (ret == -EINVAL) {
+exit(1);
}
}
#if defined(cpudef_setup)
@@ -4522,16 +4514,12 @@ int main(int argc, char **argv, char **envp)
#endif
case QEMU_OPTION_readconfig:
{
-FILE *fp;
-fp = fopen(optarg, "r");
-if (fp == NULL) {
-fprintf(stderr, "open %s: %s\n", optarg,
strerror(errno));
+int ret = qemu_read_config_file(optarg);
+if (ret < 0) {
+fprintf(stderr, "read config %s: %s\n", optarg,
+strerror(-ret));
exit(1);
}
-if (qemu_config_parse(fp, optarg) != 0) {
-exit(1);
-}
-fclose(fp);
break;
}
case QEMU_OPTION_writeconfig:
--
1.6.6.1
[Qemu-devel] [PATCH 3/7] blkdebug: Basic request passthrough
This isn't doing anything interesting. It creates the blkdebug block driver as
a protocol which just passes everything through to raw.
Signed-off-by: Kevin Wolf
---
Makefile.objs|2 +-
block/blkdebug.c | 104 ++
2 files changed, 105 insertions(+), 1 deletions(-)
create mode 100644 block/blkdebug.c
diff --git a/Makefile.objs b/Makefile.objs
index e791dd5..ece02d5 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -14,7 +14,7 @@ block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
block-nested-y += cow.o qcow.o vdi.o vmdk.o cloop.o dmg.o bochs.o vpc.o vvfat.o
block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o
-block-nested-y += parallels.o nbd.o
+block-nested-y += parallels.o nbd.o blkdebug.o
block-nested-$(CONFIG_WIN32) += raw-win32.o
block-nested-$(CONFIG_POSIX) += raw-posix.o
block-nested-$(CONFIG_CURL) += curl.o
diff --git a/block/blkdebug.c b/block/blkdebug.c
new file mode 100644
index 000..2c7e0dd
--- /dev/null
+++ b/block/blkdebug.c
@@ -0,0 +1,104 @@
+/*
+ * Block protocol for I/O error injection
+ *
+ * Copyright (c) 2010 Kevin Wolf
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu-common.h"
+#include "block_int.h"
+#include "module.h"
+
+typedef struct BDRVBlkdebugState {
+BlockDriverState *hd;
+} BDRVBlkdebugState;
+
+static int blkdebug_open(BlockDriverState *bs, const char *filename, int flags)
+{
+BDRVBlkdebugState *s = bs->opaque;
+
+if (strncmp(filename, "blkdebug:", strlen("blkdebug:"))) {
+return -EINVAL;
+}
+filename += strlen("blkdebug:");
+
+return bdrv_file_open(&s->hd, filename, flags);
+}
+
+static BlockDriverAIOCB *blkdebug_aio_readv(BlockDriverState *bs,
+int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
+BlockDriverCompletionFunc *cb, void *opaque)
+{
+BDRVBlkdebugState *s = bs->opaque;
+BlockDriverAIOCB *acb =
+bdrv_aio_readv(s->hd, sector_num, qiov, nb_sectors, cb, opaque);
+return acb;
+}
+
+static BlockDriverAIOCB *blkdebug_aio_writev(BlockDriverState *bs,
+int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
+BlockDriverCompletionFunc *cb, void *opaque)
+{
+BDRVBlkdebugState *s = bs->opaque;
+BlockDriverAIOCB *acb =
+bdrv_aio_writev(s->hd, sector_num, qiov, nb_sectors, cb, opaque);
+return acb;
+}
+
+static void blkdebug_close(BlockDriverState *bs)
+{
+BDRVBlkdebugState *s = bs->opaque;
+bdrv_delete(s->hd);
+}
+
+static void blkdebug_flush(BlockDriverState *bs)
+{
+BDRVBlkdebugState *s = bs->opaque;
+bdrv_flush(s->hd);
+}
+
+static BlockDriverAIOCB *blkdebug_aio_flush(BlockDriverState *bs,
+BlockDriverCompletionFunc *cb, void *opaque)
+{
+BDRVBlkdebugState *s = bs->opaque;
+return bdrv_aio_flush(s->hd, cb, opaque);
+}
+
+static BlockDriver bdrv_blkdebug = {
+.format_name= "blkdebug",
+.protocol_name = "blkdebug",
+
+.instance_size = sizeof(BDRVBlkdebugState),
+
+.bdrv_open = blkdebug_open,
+.bdrv_close = blkdebug_close,
+.bdrv_flush = blkdebug_flush,
+
+.bdrv_aio_readv = blkdebug_aio_readv,
+.bdrv_aio_writev= blkdebug_aio_writev,
+.bdrv_aio_flush = blkdebug_aio_flush,
+};
+
+static void bdrv_blkdebug_init(void)
+{
+bdrv_register(&bdrv_blkdebug);
+}
+
+block_init(bdrv_blkdebug_init);
--
1.6.6.1
[Qemu-devel] [PATCH 2/7] qemu-config: Make qemu_config_parse more generic
qemu_config_parse gets the option groups as a parameter now instead of
hardcoding the VM configuration groups. This way it can be used for other
configurations, too.
Signed-off-by: Kevin Wolf
---
qemu-config.c | 18 --
qemu-config.h |2 +-
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index 57dc9a5..dedf177 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -296,7 +296,7 @@ QemuOptsList qemu_cpudef_opts = {
},
};
-static QemuOptsList *lists[] = {
+static QemuOptsList *vm_config_groups[] = {
&qemu_drive_opts,
&qemu_chardev_opts,
&qemu_device_opts,
@@ -309,7 +309,7 @@ static QemuOptsList *lists[] = {
NULL,
};
-QemuOptsList *qemu_find_opts(const char *group)
+static QemuOptsList *find_list(QemuOptsList **lists, const char *group)
{
int i;
@@ -323,6 +323,11 @@ QemuOptsList *qemu_find_opts(const char *group)
return lists[i];
}
+QemuOptsList *qemu_find_opts(const char *group)
+{
+return find_list(vm_config_groups, group);
+}
+
int qemu_set_option(const char *str)
{
char group[64], id[64], arg[64];
@@ -421,6 +426,7 @@ static int config_write_opts(QemuOpts *opts, void *opaque)
void qemu_config_write(FILE *fp)
{
struct ConfigWriteData data = { .fp = fp };
+QemuOptsList **lists = vm_config_groups;
int i;
fprintf(fp, "# qemu config file\n\n");
@@ -430,7 +436,7 @@ void qemu_config_write(FILE *fp)
}
}
-int qemu_config_parse(FILE *fp, const char *fname)
+int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname)
{
char line[1024], group[64], id[64], arg[64], value[1024];
Location loc;
@@ -451,7 +457,7 @@ int qemu_config_parse(FILE *fp, const char *fname)
}
if (sscanf(line, "[%63s \"%63[^\"]\"]", group, id) == 2) {
/* group with id */
-list = qemu_find_opts(group);
+list = find_list(lists, group);
if (list == NULL)
goto out;
opts = qemu_opts_create(list, id, 1);
@@ -459,7 +465,7 @@ int qemu_config_parse(FILE *fp, const char *fname)
}
if (sscanf(line, "[%63[^]]]", group) == 1) {
/* group without id */
-list = qemu_find_opts(group);
+list = find_list(lists, group);
if (list == NULL)
goto out;
opts = qemu_opts_create(list, NULL, 0);
@@ -497,7 +503,7 @@ int qemu_read_config_file(const char *filename)
return -errno;
}
-if (qemu_config_parse(f, filename) != 0) {
+if (qemu_config_parse(f, vm_config_groups, filename) != 0) {
return -EINVAL;
}
fclose(f);
diff --git a/qemu-config.h b/qemu-config.h
index 07a7a27..dd299c2 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -17,7 +17,7 @@ int qemu_global_option(const char *str);
void qemu_add_globals(void);
void qemu_config_write(FILE *fp);
-int qemu_config_parse(FILE *fp, const char *fname);
+int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname);
int qemu_read_config_file(const char *filename);
--
1.6.6.1
[Qemu-devel] [PATCH 0/7] blkdebug
This patch series introduces a new block driver which acts as a protocol and whose purpose it is to fail requests. To be more precise, I want it to fail in configurable places, so that qemu-iotests can be extended with tests for the error paths (for example for the case when something with metadata writes goes wrong deep in qcow2). It works like this (I think this is self-explanatory): $ cat /tmp/blkdebug.cfg [inject-error] event = "l1_update" errno = "5" immediately = "on" $ qemu-io blkdebug:/tmp/blkdebug.cfg:/tmp/empty.qcow2 qemu-io> read 0 4k read 4096/4096 bytes at offset 0 4 KiB, 1 ops; 0. sec (195.312 MiB/sec and 5. ops/sec) qemu-io> write 0 4k write failed: Input/output error Changes in comparison to the RFC: - Had to rebase qemu-config changes - Code cleanup (including resolving TODOs and FIXMEs) - More events all over qcow2 - No debug messages on stderr any more Kevin Wolf (7): qemu-config: qemu_read_config_file() reads the normal config file qemu-config: Make qemu_config_parse more generic blkdebug: Basic request passthrough blkdebug: Inject errors Make qemu-config available for tools blkdebug: Add events and rules qcow2: Trigger blkdebug events Makefile.objs |6 +- block.c| 12 ++ block.h| 53 ++ block/blkdebug.c | 474 block/qcow2-cluster.c | 15 ++ block/qcow2-refcount.c | 18 ++ block/qcow2.c |6 + block_int.h|2 + hw/qdev-properties.c | 19 ++- hw/qdev.h |1 - qemu-config.c | 48 +++--- qemu-config.h |4 +- qemu-tool.c|4 + vl.c | 38 ++--- 14 files changed, 647 insertions(+), 53 deletions(-) create mode 100644 block/blkdebug.c
[Qemu-devel] [PATCH v2 09/11] error: Convert net_client_init() to QError
The conversion is shallow: client type init() methods aren't
converted. Converting them is a big job for relatively little
practical benefit, so leave it for later.
Signed-off-by: Markus Armbruster
---
net.c | 38 ++
1 files changed, 18 insertions(+), 20 deletions(-)
diff --git a/net.c b/net.c
index 9338f35..1f3c39c 100644
--- a/net.c
+++ b/net.c
@@ -1057,18 +1057,12 @@ int net_client_init(Monitor *mon, QemuOpts *opts, int
is_netdev)
int i;
type = qemu_opt_get(opts, "type");
+if (!type) {
+qerror_report(QERR_MISSING_PARAMETER, "type");
+return -1;
+}
-if (!is_netdev) {
-if (!type) {
-error_report("No type specified for -net");
-return -1;
-}
-} else {
-if (!type) {
-error_report("No type specified for -netdev");
-return -1;
-}
-
+if (is_netdev) {
if (strcmp(type, "tap") != 0 &&
#ifdef CONFIG_SLIRP
strcmp(type, "user") != 0 &&
@@ -1077,21 +1071,21 @@ int net_client_init(Monitor *mon, QemuOpts *opts, int
is_netdev)
strcmp(type, "vde") != 0 &&
#endif
strcmp(type, "socket") != 0) {
-error_report("The '%s' network backend type is not valid with
-netdev",
- type);
+qerror_report(QERR_INVALID_PARAMETER_VALUE, "type",
+ "a netdev backend type");
return -1;
}
if (qemu_opt_get(opts, "vlan")) {
-error_report("The 'vlan' parameter is not valid with -netdev");
+qerror_report(QERR_INVALID_PARAMETER, "vlan");
return -1;
}
if (qemu_opt_get(opts, "name")) {
-error_report("The 'name' parameter is not valid with -netdev");
+qerror_report(QERR_INVALID_PARAMETER, "name");
return -1;
}
if (!qemu_opts_id(opts)) {
-error_report("The id= parameter is required with -netdev");
+qerror_report(QERR_MISSING_PARAMETER, "id");
return -1;
}
}
@@ -1117,14 +,18 @@ int net_client_init(Monitor *mon, QemuOpts *opts, int
is_netdev)
}
if (net_client_types[i].init) {
-return net_client_types[i].init(opts, mon, name, vlan);
-} else {
-return 0;
+if (net_client_types[i].init(opts, mon, name, vlan) < 0) {
+/* TODO push error reporting into init() methods */
+qerror_report(QERR_DEVICE_INIT_FAILED, type);
+return -1;
+}
}
+return 0;
}
}
-error_report("Invalid -net type '%s'", type);
+qerror_report(QERR_INVALID_PARAMETER_VALUE, "type",
+ "a network backend type");
return -1;
}
--
1.6.6.1
[Qemu-devel] [PATCH v2 02/11] error: New QERR_DUPLICATE_ID
Signed-off-by: Markus Armbruster
---
qerror.c |4
qerror.h |3 +++
2 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/qerror.c b/qerror.c
index 4520b0d..9fb817e 100644
--- a/qerror.c
+++ b/qerror.c
@@ -97,6 +97,10 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Device '%(device)' has no child bus",
},
{
+.error_fmt = QERR_DUPLICATE_ID,
+.desc = "Duplicate ID '%(id)' for %(object)",
+},
+{
.error_fmt = QERR_FD_NOT_FOUND,
.desc = "File descriptor named '%(name)' not found",
},
diff --git a/qerror.h b/qerror.h
index a2664ab..870cdc3 100644
--- a/qerror.h
+++ b/qerror.h
@@ -88,6 +88,9 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_DEVICE_NO_BUS \
"{ 'class': 'DeviceNoBus', 'data': { 'device': %s } }"
+#define QERR_DUPLICATE_ID \
+"{ 'class': 'DuplicateId', 'data': { 'id': %s, 'object': %s } }"
+
#define QERR_FD_NOT_FOUND \
"{ 'class': 'FdNotFound', 'data': { 'name': %s } }"
--
1.6.6.1
[Qemu-devel] [PATCH v2 00/11] monitor: New commands netdev_add, netdev_del
v2: Avoid loss of help in parse_option_size(). Rebased. Markus Armbruster (11): error: Put error definitions back in alphabetical order error: New QERR_DUPLICATE_ID error: Convert qemu_opts_create() to QError error: New QERR_INVALID_PARAMETER_VALUE error: Convert qemu_opts_set() to QError error: Drop extra messages after qemu_opts_set() and qemu_opts_parse() error: Use QERR_INVALID_PARAMETER_VALUE instead of QERR_INVALID_PARAMETER error: Convert qemu_opts_validate() to QError error: Convert net_client_init() to QError error: New QERR_DEVICE_IN_USE monitor: New commands netdev_add, netdev_del hw/pci-hotplug.c |2 - hw/qdev.c|2 +- monitor.c|6 ++- net.c| 95 + net.h|2 + qemu-config.c|1 - qemu-monitor.hx | 30 + qemu-option.c| 24 ++ qerror.c | 20 ++- qerror.h | 17 -- vl.c |5 --- 11 files changed, 152 insertions(+), 52 deletions(-)
[Qemu-devel] [PATCH v2 03/11] error: Convert qemu_opts_create() to QError
Fixes device_add to report duplicate ID properly in QMP, as
DuplicateId instead of UndefinedError.
Signed-off-by: Markus Armbruster
---
qemu-option.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/qemu-option.c b/qemu-option.c
index f83d07c..12ce322 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -30,6 +30,7 @@
#include "qemu-error.h"
#include "qemu-objects.h"
#include "qemu-option.h"
+#include "qerror.h"
/*
* Extracts the name of an option from the parameter string (p points at the
@@ -643,8 +644,7 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char
*id, int fail_if_exist
opts = qemu_opts_find(list, id);
if (opts != NULL) {
if (fail_if_exists) {
-fprintf(stderr, "tried to create id \"%s\" twice for \"%s\"\n",
-id, list->name);
+qerror_report(QERR_DUPLICATE_ID, id, list->name);
return NULL;
} else {
return opts;
--
1.6.6.1
[Qemu-devel] [PATCH v2 08/11] error: Convert qemu_opts_validate() to QError
Signed-off-by: Markus Armbruster
---
qemu-option.c |3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/qemu-option.c b/qemu-option.c
index 394c763..1ffc497 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -877,8 +877,7 @@ int qemu_opts_validate(QemuOpts *opts, const QemuOptDesc
*desc)
}
}
if (desc[i].name == NULL) {
-fprintf(stderr, "option \"%s\" is not valid for %s\n",
-opt->name, opts->list->name);
+qerror_report(QERR_INVALID_PARAMETER, opt->name);
return -1;
}
--
1.6.6.1
[Qemu-devel] [PATCH v2 05/11] error: Convert qemu_opts_set() to QError
Signed-off-by: Markus Armbruster
---
qemu-option.c | 17 +++--
1 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/qemu-option.c b/qemu-option.c
index 12ce322..394c763 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -176,7 +176,7 @@ static int parse_option_bool(const char *name, const char
*value, int *ret)
} else if (!strcmp(value, "off")) {
*ret = 0;
} else {
-fprintf(stderr, "Option '%s': Use 'on' or 'off'\n", name);
+qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "'on' or 'off'");
return -1;
}
} else {
@@ -193,12 +193,12 @@ static int parse_option_number(const char *name, const
char *value, uint64_t *re
if (value != NULL) {
number = strtoull(value, &postfix, 0);
if (*postfix != '\0') {
-fprintf(stderr, "Option '%s' needs a number as parameter\n", name);
+qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a number");
return -1;
}
*ret = number;
} else {
-fprintf(stderr, "Option '%s' needs a parameter\n", name);
+qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a number");
return -1;
}
return 0;
@@ -226,13 +226,13 @@ static int parse_option_size(const char *name, const char
*value, uint64_t *ret)
*ret = (uint64_t) sizef;
break;
default:
-fprintf(stderr, "Option '%s' needs size as parameter\n", name);
-fprintf(stderr, "You may use k, M, G or T suffixes for "
+qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a size");
+error_printf_unless_qmp("You may use k, M, G or T suffixes for "
"kilobytes, megabytes, gigabytes and terabytes.\n");
return -1;
}
} else {
-fprintf(stderr, "Option '%s' needs a parameter\n", name);
+qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a size");
return -1;
}
return 0;
@@ -581,8 +581,7 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const
char *value)
if (i == 0) {
/* empty list -> allow any */;
} else {
-fprintf(stderr, "option \"%s\" is not valid for %s\n",
-name, opts->list->name);
+qerror_report(QERR_INVALID_PARAMETER, name);
return -1;
}
}
@@ -598,8 +597,6 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const
char *value)
opt->str = qemu_strdup(value);
}
if (qemu_opt_parse(opt) < 0) {
-fprintf(stderr, "Failed to parse \"%s\" for \"%s.%s\"\n", opt->str,
-opts->list->name, opt->name);
qemu_opt_del(opt);
return -1;
}
--
1.6.6.1
[Qemu-devel] [PATCH v2 07/11] error: Use QERR_INVALID_PARAMETER_VALUE instead of QERR_INVALID_PARAMETER
Signed-off-by: Markus Armbruster
---
hw/qdev.c |2 +-
monitor.c |6 --
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index 17a46a7..f45ed0f 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -207,7 +207,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
/* find driver */
info = qdev_find_info(NULL, driver);
if (!info || info->no_user) {
-qerror_report(QERR_INVALID_PARAMETER, "driver");
+qerror_report(QERR_INVALID_PARAMETER_VALUE, "driver", "a driver name");
error_printf_unless_qmp("Try with argument '?' for a list.\n");
return NULL;
}
diff --git a/monitor.c b/monitor.c
index 822dc27..35cbce7 100644
--- a/monitor.c
+++ b/monitor.c
@@ -970,7 +970,8 @@ static int do_cpu_set(Monitor *mon, const QDict *qdict,
QObject **ret_data)
{
int index = qdict_get_int(qdict, "index");
if (mon_set_cpu(index) < 0) {
-qerror_report(QERR_INVALID_PARAMETER, "index");
+qerror_report(QERR_INVALID_PARAMETER_VALUE, "index",
+ "a CPU number");
return -1;
}
return 0;
@@ -2489,7 +2490,8 @@ static int do_getfd(Monitor *mon, const QDict *qdict,
QObject **ret_data)
}
if (qemu_isdigit(fdname[0])) {
-qerror_report(QERR_INVALID_PARAMETER, "fdname");
+qerror_report(QERR_INVALID_PARAMETER_VALUE, "fdname",
+ "a name not starting with a digit");
return -1;
}
--
1.6.6.1
[Qemu-devel] [PATCH v2 11/11] monitor: New commands netdev_add, netdev_del
Monitor commands to go with -netdev.
Signed-off-by: Markus Armbruster
---
net.c | 57 ++-
net.h |2 +
qemu-monitor.hx | 30
3 files changed, 88 insertions(+), 1 deletions(-)
diff --git a/net.c b/net.c
index 1f3c39c..80e9025 100644
--- a/net.c
+++ b/net.c
@@ -1122,7 +1122,7 @@ int net_client_init(Monitor *mon, QemuOpts *opts, int
is_netdev)
}
qerror_report(QERR_INVALID_PARAMETER_VALUE, "type",
- "a network backend type");
+ "a network client type");
return -1;
}
@@ -1186,6 +1186,61 @@ void net_host_device_remove(Monitor *mon, const QDict
*qdict)
qemu_del_vlan_client(vc);
}
+/**
+ * do_netdev_add(): Add a host network device
+ *
+ * Argument qdict contains
+ * - "type": the device type, "tap", "user", ...
+ * - "id": the device's ID (must be unique)
+ * - device options
+ *
+ * Example:
+ *
+ * { "type": "user", "id": "netdev1", "hostname": "a-guest" }
+ */
+int do_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+QemuOpts *opts;
+int res;
+
+opts = qemu_opts_from_qdict(&qemu_netdev_opts, qdict);
+if (!opts) {
+return -1;
+}
+
+res = net_client_init(mon, opts, 1);
+qemu_opts_del(opts);
+return res;
+}
+
+/**
+ * do_netdev_del(): Delete a host network device
+ *
+ * Argument qdict contains
+ * - "id": the device's ID
+ *
+ * Example:
+ *
+ * { "id": "netdev1" }
+ */
+int do_netdev_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+const char *id = qdict_get_str(qdict, "id");
+VLANClientState *vc;
+
+vc = qemu_find_netdev(id);
+if (!vc || vc->info->type == NET_CLIENT_TYPE_NIC) {
+qerror_report(QERR_DEVICE_NOT_FOUND, id);
+return -1;
+}
+if (vc->peer) {
+qerror_report(QERR_DEVICE_IN_USE, id);
+return -1;
+}
+qemu_del_vlan_client(vc);
+return 0;
+}
+
void net_set_boot_mask(int net_boot_mask)
{
int i;
diff --git a/net.h b/net.h
index 16f19c5..ce9e2c6 100644
--- a/net.h
+++ b/net.h
@@ -166,6 +166,8 @@ void net_cleanup(void);
void net_set_boot_mask(int boot_mask);
void net_host_device_add(Monitor *mon, const QDict *qdict);
void net_host_device_remove(Monitor *mon, const QDict *qdict);
+int do_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
+int do_netdev_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
#define DEFAULT_NETWORK_SCRIPT "/etc/qemu-ifup"
#define DEFAULT_NETWORK_DOWN_SCRIPT "/etc/qemu-ifdown"
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 5308f36..ff5f099 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -913,6 +913,36 @@ STEXI
Remove host VLAN client.
ETEXI
+{
+.name = "netdev_add",
+.args_type = "netdev:O",
+.params = "[user|tap|socket],id=str[,prop=value][,...]",
+.help = "add host network device",
+.user_print = monitor_user_noop,
+.mhandler.cmd_new = do_netdev_add,
+},
+
+STEXI
+...@item netdev_add
+...@findex netdev_add
+Add host network device.
+ETEXI
+
+{
+.name = "netdev_del",
+.args_type = "id:s",
+.params = "id",
+.help = "remove host network device",
+.user_print = monitor_user_noop,
+.mhandler.cmd_new = do_netdev_del,
+},
+
+STEXI
+...@item netdev_del
+...@findex netdev_del
+Remove host network device.
+ETEXI
+
#ifdef CONFIG_SLIRP
{
.name = "hostfwd_add",
--
1.6.6.1
[Qemu-devel] [PATCH v2 10/11] error: New QERR_DEVICE_IN_USE
Signed-off-by: Markus Armbruster
---
qerror.c |4
qerror.h |3 +++
2 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/qerror.c b/qerror.c
index 97e8d4a..8d885cd 100644
--- a/qerror.c
+++ b/qerror.c
@@ -69,6 +69,10 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Device '%(device)' could not be initialized",
},
{
+.error_fmt = QERR_DEVICE_IN_USE,
+.desc = "Device '%(device)' is in use",
+},
+{
.error_fmt = QERR_DEVICE_LOCKED,
.desc = "Device '%(device)' is locked",
},
diff --git a/qerror.h b/qerror.h
index 5625d54..bae08c0 100644
--- a/qerror.h
+++ b/qerror.h
@@ -67,6 +67,9 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_DEVICE_INIT_FAILED \
"{ 'class': 'DeviceInitFailed', 'data': { 'device': %s } }"
+#define QERR_DEVICE_IN_USE \
+"{ 'class': 'DeviceInUse', 'data': { 'device': %s } }"
+
#define QERR_DEVICE_LOCKED \
"{ 'class': 'DeviceLocked', 'data': { 'device': %s } }"
--
1.6.6.1
[Qemu-devel] [PATCH v2 06/11] error: Drop extra messages after qemu_opts_set() and qemu_opts_parse()
Both functions report errors nicely enough now, no need for additional
messages.
Signed-off-by: Markus Armbruster
---
hw/pci-hotplug.c |2 --
net.c|2 --
qemu-config.c|1 -
vl.c |5 -
4 files changed, 0 insertions(+), 10 deletions(-)
diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index eb3701b..343fd17 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -56,8 +56,6 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
opts = qemu_opts_parse(&qemu_net_opts, opts_str ? opts_str : "", 0);
if (!opts) {
-monitor_printf(mon, "parsing network options '%s' failed\n",
- opts_str ? opts_str : "");
return NULL;
}
diff --git a/net.c b/net.c
index 1092bdc..9338f35 100644
--- a/net.c
+++ b/net.c
@@ -1161,8 +1161,6 @@ void net_host_device_add(Monitor *mon, const QDict *qdict)
opts = qemu_opts_parse(&qemu_net_opts, opts_str ? opts_str : "", 0);
if (!opts) {
-monitor_printf(mon, "parsing network options '%s' failed\n",
- opts_str ? opts_str : "");
return;
}
diff --git a/qemu-config.c b/qemu-config.c
index 150157c..d4a2f43 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -472,7 +472,6 @@ int qemu_config_parse(FILE *fp, const char *fname)
goto out;
}
if (qemu_opt_set(opts, arg, value) != 0) {
-error_report("failed to set \"%s\" for %s", arg, group);
goto out;
}
continue;
diff --git a/vl.c b/vl.c
index d69250c..35ab34f 100644
--- a/vl.c
+++ b/vl.c
@@ -766,8 +766,6 @@ QemuOpts *drive_add(const char *file, const char *fmt, ...)
opts = qemu_opts_parse(&qemu_drive_opts, optstr, 0);
if (!opts) {
-fprintf(stderr, "%s: huh? duplicate? (%s)\n",
-__FUNCTION__, optstr);
return NULL;
}
if (file)
@@ -4244,7 +4242,6 @@ int main(int argc, char **argv, char **envp)
case QEMU_OPTION_mon:
opts = qemu_opts_parse(&qemu_mon_opts, optarg, 1);
if (!opts) {
-fprintf(stderr, "parse error: %s\n", optarg);
exit(1);
}
default_monitor = 0;
@@ -4252,7 +4249,6 @@ int main(int argc, char **argv, char **envp)
case QEMU_OPTION_chardev:
opts = qemu_opts_parse(&qemu_chardev_opts, optarg, 1);
if (!opts) {
-fprintf(stderr, "parse error: %s\n", optarg);
exit(1);
}
break;
@@ -4464,7 +4460,6 @@ int main(int argc, char **argv, char **envp)
case QEMU_OPTION_rtc:
opts = qemu_opts_parse(&qemu_rtc_opts, optarg, 0);
if (!opts) {
-fprintf(stderr, "parse error: %s\n", optarg);
exit(1);
}
configure_rtc(opts);
--
1.6.6.1
[Qemu-devel] [PATCH v2 04/11] error: New QERR_INVALID_PARAMETER_VALUE
Signed-off-by: Markus Armbruster
---
qerror.c |4
qerror.h |3 +++
2 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/qerror.c b/qerror.c
index 9fb817e..97e8d4a 100644
--- a/qerror.c
+++ b/qerror.c
@@ -121,6 +121,10 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Invalid parameter type, expected: %(expected)",
},
{
+.error_fmt = QERR_INVALID_PARAMETER_VALUE,
+.desc = "Parameter '%(name)' expects %(expected)",
+},
+{
.error_fmt = QERR_INVALID_PASSWORD,
.desc = "Password incorrect",
},
diff --git a/qerror.h b/qerror.h
index 870cdc3..5625d54 100644
--- a/qerror.h
+++ b/qerror.h
@@ -106,6 +106,9 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_INVALID_PARAMETER_TYPE \
"{ 'class': 'InvalidParameterType', 'data': { 'name': %s,'expected': %s }
}"
+#define QERR_INVALID_PARAMETER_VALUE \
+"{ 'class': 'InvalidParameterValue', 'data': { 'name': %s, 'expected': %s
} }"
+
#define QERR_INVALID_PASSWORD \
"{ 'class': 'InvalidPassword', 'data': {} }"
--
1.6.6.1
[Qemu-devel] [PATCH v2 01/11] error: Put error definitions back in alphabetical order
Add suitable comments to help keerp them in order.
Signed-off-by: Markus Armbruster
---
qerror.c | 12
qerror.h |8 +---
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/qerror.c b/qerror.c
index eaa1deb..4520b0d 100644
--- a/qerror.c
+++ b/qerror.c
@@ -38,6 +38,10 @@ static const QType qerror_type = {
* for example:
*
* "running out of foo: %(foo)%%"
+ *
+ * Please keep the entries in alphabetical order.
+ * Use "sed -n '/^static.*qerror_table\[\]/,/^};/s/QERR_/&/gp' qerror.c | sort
-c"
+ * to check.
*/
static const QErrorStringTable qerror_table[] = {
{
@@ -65,10 +69,6 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Device '%(device)' could not be initialized",
},
{
-.error_fmt = QERR_DEVICE_NOT_ENCRYPTED,
-.desc = "Device '%(device)' is not encrypted",
-},
-{
.error_fmt = QERR_DEVICE_LOCKED,
.desc = "Device '%(device)' is locked",
},
@@ -81,6 +81,10 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Device '%(device)' has not been activated by the guest",
},
{
+.error_fmt = QERR_DEVICE_NOT_ENCRYPTED,
+.desc = "Device '%(device)' is not encrypted",
+},
+{
.error_fmt = QERR_DEVICE_NOT_FOUND,
.desc = "Device '%(device)' not found",
},
diff --git a/qerror.h b/qerror.h
index dd298d4..a2664ab 100644
--- a/qerror.h
+++ b/qerror.h
@@ -46,6 +46,8 @@ QError *qobject_to_qerror(const QObject *obj);
/*
* QError class list
+ * Please keep the definitions in alphabetical order.
+ * Use "grep '^#define QERR_' qerror.h | sort -c" to check.
*/
#define QERR_BAD_BUS_FOR_DEVICE \
"{ 'class': 'BadBusForDevice', 'data': { 'device': %s, 'bad_bus_type': %s
} }"
@@ -62,9 +64,6 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_DEVICE_ENCRYPTED \
"{ 'class': 'DeviceEncrypted', 'data': { 'device': %s } }"
-#define QERR_DEVICE_NOT_ENCRYPTED \
-"{ 'class': 'DeviceNotEncrypted', 'data': { 'device': %s } }"
-
#define QERR_DEVICE_INIT_FAILED \
"{ 'class': 'DeviceInitFailed', 'data': { 'device': %s } }"
@@ -77,6 +76,9 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_DEVICE_NOT_ACTIVE \
"{ 'class': 'DeviceNotActive', 'data': { 'device': %s } }"
+#define QERR_DEVICE_NOT_ENCRYPTED \
+"{ 'class': 'DeviceNotEncrypted', 'data': { 'device': %s } }"
+
#define QERR_DEVICE_NOT_FOUND \
"{ 'class': 'DeviceNotFound', 'data': { 'device': %s } }"
--
1.6.6.1
Re: [Qemu-devel] [PATCH 5/6] error: Link qemu-img, qemu-nbd, qemu-io with qemu-error.o
Blue Swirl writes:
> On 3/18/10, Markus Armbruster wrote:
>> Blue Swirl writes:
>>
>> > On 3/18/10, Markus Armbruster wrote:
>> >> Blue Swirl writes:
>> >>
>> >> > On 3/17/10, Markus Armbruster wrote:
>> >> >> Blue Swirl writes:
>> >> >>
>> >> >> > On 3/17/10, Markus Armbruster wrote:
>> >>
>> >> [...]
>> >>
>> >> >> >> +void monitor_set_error(Monitor *mon, QError *qerror)
>> >> >> >> +{
>> >> >> >> +assert(0);
>> >> >> >
>> >> >> > Please use abort().
>> >> >>
>> >> >>
>> >> >> Why?
>> >> >
>> >> > Because assert(0) does not abort when compiled with -DNDEBUG.
>> >>
>> >>
>> >> Why is that a problem? And why isn't it a problem for the 300+ other
>> >> assertions in the code?
>> >
>> > We didn't support -DNDEBUG build until recently. Therefore it's safe
>> > to assume that also on production builds assert(0) was equal to
>> > abort(). If the default for production builds changes to -DNDEBUG, we
>> > want to retain the same abort() behaviour.
>> >
>> > The assertions which perform debugging checks aren't a problem. But
>> > assert(0) clearly isn't a debugging check, if you ever reach this line
>> > of code, it's abort() time.
>>
>>
>> I *know* that this line cannot be reached. That's why I asserted it
>> cannot be reached.
>>
>> If you want "this can't ever happen" assertions to be checked, then you
>> shouldn't define NDEBUG.
>
> If you know for sure that this line can't be reached, why bother with
> any code at all?
>
>> Do you still want me to use abort() here?
>
> abort() or nothing at all, as you wish.
Okay, nothing it is.
>> By the way, a quick grep shows >50 uses of assert(0).
>
> Not anymore since 43dc2a645e00e6761a741e3d16c27c5b5a373b66.
I doubt the wisdom of such a wholesale conversion, but I it's hardly
worth arguing now.
[Qemu-devel] [PATCH v2 6/6] error: Move qerror_report() from qemu-error.[ch] to qerror.[ch]
Signed-off-by: Markus Armbruster
---
qemu-error.c | 18 --
qemu-error.h |6 --
qerror.c | 20
qerror.h |5 +
4 files changed, 25 insertions(+), 24 deletions(-)
diff --git a/qemu-error.c b/qemu-error.c
index 9b9c0a1..57d7555 100644
--- a/qemu-error.c
+++ b/qemu-error.c
@@ -207,21 +207,3 @@ void error_report(const char *fmt, ...)
va_end(ap);
error_printf("\n");
}
-
-void qerror_report_internal(const char *file, int linenr, const char *func,
-const char *fmt, ...)
-{
-va_list va;
-QError *qerror;
-
-va_start(va, fmt);
-qerror = qerror_from_info(file, linenr, func, fmt, &va);
-va_end(va);
-
-if (monitor_cur_is_qmp()) {
-monitor_set_error(cur_mon, qerror);
-} else {
-qerror_print(qerror);
-QDECREF(qerror);
-}
-}
diff --git a/qemu-error.h b/qemu-error.h
index e63c6ab..a45609f 100644
--- a/qemu-error.h
+++ b/qemu-error.h
@@ -37,11 +37,5 @@ void error_printf_unless_qmp(const char *fmt, ...)
void error_print_loc(void);
void error_set_progname(const char *argv0);
void error_report(const char *fmt, ...) __attribute__ ((format(printf, 1, 2)));
-void qerror_report_internal(const char *file, int linenr, const char *func,
-const char *fmt, ...)
-__attribute__ ((format(printf, 4, 5)));
-
-#define qerror_report(fmt, ...) \
-qerror_report_internal(__FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__)
#endif
diff --git a/qerror.c b/qerror.c
index ff2fbd5..eaa1deb 100644
--- a/qerror.c
+++ b/qerror.c
@@ -9,6 +9,8 @@
* This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
* See the COPYING.LIB file in the top-level directory.
*/
+
+#include "monitor.h"
#include "qjson.h"
#include "qerror.h"
#include "qemu-common.h"
@@ -377,6 +379,24 @@ void qerror_print(QError *qerror)
QDECREF(qstring);
}
+void qerror_report_internal(const char *file, int linenr, const char *func,
+const char *fmt, ...)
+{
+va_list va;
+QError *qerror;
+
+va_start(va, fmt);
+qerror = qerror_from_info(file, linenr, func, fmt, &va);
+va_end(va);
+
+if (monitor_cur_is_qmp()) {
+monitor_set_error(cur_mon, qerror);
+} else {
+qerror_print(qerror);
+QDECREF(qerror);
+}
+}
+
/**
* qobject_to_qerror(): Convert a QObject into a QError
*/
diff --git a/qerror.h b/qerror.h
index d96abe1..dd298d4 100644
--- a/qerror.h
+++ b/qerror.h
@@ -37,6 +37,11 @@ QError *qerror_from_info(const char *file, int linenr, const
char *func,
const char *fmt, va_list *va);
QString *qerror_human(const QError *qerror);
void qerror_print(QError *qerror);
+void qerror_report_internal(const char *file, int linenr, const char *func,
+const char *fmt, ...)
+__attribute__ ((format(printf, 4, 5)));
+#define qerror_report(fmt, ...) \
+qerror_report_internal(__FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__)
QError *qobject_to_qerror(const QObject *obj);
/*
--
1.6.6.1
[Qemu-devel] [PATCH v2 2/6] error: Trim includes in qerror.c
Signed-off-by: Markus Armbruster --- qerror.c |2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/qerror.c b/qerror.c index d0aba61..ff2fbd5 100644 --- a/qerror.c +++ b/qerror.c @@ -11,9 +11,7 @@ */ #include "qjson.h" #include "qerror.h" -#include "qstring.h" #include "qemu-common.h" -#include "qemu-error.h" static void qerror_destroy_obj(QObject *obj); -- 1.6.6.1
[Qemu-devel] [PATCH v2 1/6] error: Trim includes after "Move qemu_error & friends..."
Missed in commit 2f792016.
Signed-off-by: Markus Armbruster
---
hw/qdev-properties.c |1 +
monitor.c|2 --
sysemu.h |2 --
3 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 92d6793..157a111 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -1,6 +1,7 @@
#include "sysemu.h"
#include "net.h"
#include "qdev.h"
+#include "qerror.h"
void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
{
diff --git a/monitor.c b/monitor.c
index 0448a70..822dc27 100644
--- a/monitor.c
+++ b/monitor.c
@@ -49,10 +49,8 @@
#include "qint.h"
#include "qfloat.h"
#include "qlist.h"
-#include "qdict.h"
#include "qbool.h"
#include "qstring.h"
-#include "qerror.h"
#include "qjson.h"
#include "json-streamer.h"
#include "json-parser.h"
diff --git a/sysemu.h b/sysemu.h
index 525efd1..5464d35 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -6,8 +6,6 @@
#include "qemu-option.h"
#include "qemu-queue.h"
#include "qemu-timer.h"
-#include "qdict.h"
-#include "qerror.h"
#ifdef _WIN32
#include
--
1.6.6.1
[Qemu-devel] [PATCH v2 5/6] error: Link qemu-img, qemu-nbd, qemu-io with qemu-error.o
The location tracking interface is used by code shared with qemi-img,
qemu-nbd and qemu-io, so it needs to be available there. Commit
827b0813 provides it in a rather hamfisted way: it adds a dummy
implementation to qemu-tool.c.
It's cleaner to provide the real thing, and put a few more dummy
monitor functions into qemu-tool.c.
Signed-off-by: Markus Armbruster
---
Makefile|6 +++---
qemu-tool.c | 49 +++--
2 files changed, 18 insertions(+), 37 deletions(-)
diff --git a/Makefile b/Makefile
index 57c354d..a404fda 100644
--- a/Makefile
+++ b/Makefile
@@ -129,11 +129,11 @@ bt-host.o: QEMU_CFLAGS += $(BLUEZ_CFLAGS)
qemu-img.o: qemu-img-cmds.h
qemu-img.o qemu-tool.o qemu-nbd.o qemu-io.o: $(GENERATED_HEADERS)
-qemu-img$(EXESUF): qemu-img.o qemu-tool.o $(block-obj-y) $(qobject-obj-y)
+qemu-img$(EXESUF): qemu-img.o qemu-tool.o qemu-error.o $(block-obj-y)
$(qobject-obj-y)
-qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o $(block-obj-y) $(qobject-obj-y)
+qemu-nbd$(EXESUF): qemu-nbd.o qemu-tool.o qemu-error.o $(block-obj-y)
$(qobject-obj-y)
-qemu-io$(EXESUF): qemu-io.o qemu-tool.o cmd.o $(block-obj-y) $(qobject-obj-y)
+qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(block-obj-y)
$(qobject-obj-y)
qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
$(call quiet-command,sh $(SRC_PATH)/hxtool -h < $< > $@," GEN $@")
diff --git a/qemu-tool.c b/qemu-tool.c
index dda752b..b39af86 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -15,7 +15,6 @@
#include "monitor.h"
#include "qemu-timer.h"
#include "qemu-log.h"
-#include "qemu-error.h"
#include
@@ -33,6 +32,21 @@ void qemu_service_io(void)
{
}
+Monitor *cur_mon;
+
+int monitor_cur_is_qmp(void)
+{
+return 0;
+}
+
+void monitor_set_error(Monitor *mon, QError *qerror)
+{
+}
+
+void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
+{
+}
+
void monitor_printf(Monitor *mon, const char *fmt, ...)
{
}
@@ -103,36 +117,3 @@ int64_t qemu_get_clock(QEMUClock *clock)
qemu_gettimeofday(&tv);
return (tv.tv_sec * 10LL + (tv.tv_usec * 1000)) / 100;
}
-
-Location *loc_push_restore(Location *loc)
-{
-return loc;
-}
-
-Location *loc_push_none(Location *loc)
-{
-return loc;
-}
-
-Location *loc_pop(Location *loc)
-{
-return loc;
-}
-
-Location *loc_save(Location *loc)
-{
-return loc;
-}
-
-void loc_restore(Location *loc)
-{
-}
-
-void error_report(const char *fmt, ...)
-{
-va_list args;
-
-va_start(args, fmt);
-vfprintf(stderr, fmt, args);
-va_end(args);
-}
--
1.6.6.1
[Qemu-devel] [PATCH v2 4/6] error: Make use of error_set_progname() optional
Signed-off-by: Markus Armbruster
---
qemu-error.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/qemu-error.c b/qemu-error.c
index 5d5fe37..9b9c0a1 100644
--- a/qemu-error.c
+++ b/qemu-error.c
@@ -167,7 +167,7 @@ void error_print_loc(void)
int i;
const char *const *argp;
-if (!cur_mon) {
+if (!cur_mon && progname) {
fprintf(stderr, "%s:", progname);
sep = " ";
}
--
1.6.6.1
[Qemu-devel] [PATCH v2 0/6] error: Clean up after recent changes
Cleaner integration of location tracking with qemu-tool.c. Move qerror_report() where it belongs. v2: Remove an assertion that unreachable code can't be reached, at Blue Swirl's request. Rebased. Markus Armbruster (6): error: Trim includes after "Move qemu_error & friends..." error: Trim includes in qerror.c error: Trim includes after "Infrastructure to track locations..." error: Make use of error_set_progname() optional error: Link qemu-img, qemu-nbd, qemu-io with qemu-error.o error: Move qerror_report() from qemu-error.[ch] to qerror.[ch] Makefile |6 +++--- hw/qdev-properties.c |1 + monitor.c|2 -- monitor.h|1 - qemu-error.c | 20 +--- qemu-error.h |6 -- qemu-tool.c | 49 +++-- qerror.c | 22 -- qerror.h |5 + sysemu.h |2 -- 10 files changed, 45 insertions(+), 69 deletions(-)
[Qemu-devel] [PATCH v2 3/6] error: Trim includes after "Infrastructure to track locations..."
Missed in commit 827b0813. Signed-off-by: Markus Armbruster --- monitor.h |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/monitor.h b/monitor.h index bd4ae34..5bdeed1 100644 --- a/monitor.h +++ b/monitor.h @@ -3,7 +3,6 @@ #include "qemu-common.h" #include "qemu-char.h" -#include "qemu-error.h" #include "qerror.h" #include "qdict.h" #include "block.h" -- 1.6.6.1
Re: [Qemu-devel] Re: >2 serial ports?
On 03/22/2010 10:35 AM, Michael Tokarev wrote:
Paul Brook wrote at Wed, 17 Mar 2010 11:18:09 +:
Oh, well, yes, I remember. qemu is more strict on ISA irq sharing now.
A bit too strict.
/me goes dig out a old patch which never made it upstream for some
reason I forgot. Attached.
This is wrong. Two devices should never be manipulating the same qemu_irq
object. If you want multiple devices connected to the same IRQ then you need
an explicit multiplexer. e.g. arm_timer.c:sp804_set_irq.
So... what we have to do here?
I've looked at the mentioned routine, here it is:
/* Merge the IRQs from the two component devices. */
static void sp804_set_irq(void *opaque, int irq, int level)
{
sp804_state *s = (sp804_state *)opaque;
s->level[irq] = level;
qemu_set_irq(s->irq, s->level[0] || s->level[1]);
}
But I know nothing about qemu internals, so don't quite
understand how to do this in case of serial ports. I
see it is tracking two timers and raises the irq level
if at least one half is raised... That to say - I've
got the idea, but how to apply it to serial ports?
Two devices have the same s->irq. Give each on its own qemu_irq, and
feed it into a multiplexer that ORs them together and sends the result
to the interrupt controller's qemu_irq:
S1 qemu_irq>+--+
| mux +---qemu_irq-> irq controller
S2 qemu_irq>+--+
(the ascii art will come out all wrong, I know it)
--
Do not meddle in the internals of kernels, for they are subtle and quick to
panic.
[Qemu-devel] Re: Build always fail on x86_32 host for i386_softmmu target
Bruce Majia wrote: > Hi, > > When I built qemu on my x86_32 host with following configure line: > try to do first a: make distclean > $ ./configure --prefix=/usr/local/qemus/master \ > --target-list=i386-softmmu > $ make there are several config.h & config.mak around (from old builds) you need to remove then 1st. Later, Juan.
[Qemu-devel] Re: [PATCH 0/9] Virtio cleanups
On Mon, Mar 22, 2010 at 02:13:48AM +0100, Juan Quintela wrote: > "Michael S. Tsirkin" wrote: > > On Sun, Mar 21, 2010 at 06:11:43PM +, Jamie Lokier wrote: > >> Michael S. Tsirkin wrote: > >> > That's version 1 of my patch. Version 2 removed even need for macro > >> > completely by moving allocations to the caller. > >> > >> The downside of moving allocations are: (1) it's one more call in the > >> caller, to allocate the type, (2) it needs a virtual destructor for > >> each type to free the object, which can clutter the code if there is > >> no other reason for virtual destructors. > > > > BTW I don't understand why do you refer to virtual destructors here. > > When virtio-net.c allocates and frees structure of type VirtIONet > > this is analogous to regular destructur, not a virtual one. > > If you remove it in virtio-net.c, you are right. > > If your remove it in virtio.c, then you need the equivalent of a virtual > destructor (somehow you need to find a field that is an offset and do a > free(pointer- offset). > > If struct VirtIODevice is the 1st field of everything, then a simple > free(pointer) is enough and does the right thing. > > Notice that as just now there is no free call, you can put it in either > place. We should remove it in virtio-net.c. We need to do cleanup there anyway. > > >> I don't think those are necessarily bad, but they can remove from the > >> neatness of existing code. Personally I favour an occasional macro > >> using sizeof/offsetof/container_of if the result is a natural and > >> sensible API to all of its callers. > >> > >> -- Jamie > > Later, Juan.
[Qemu-devel] Re: [PATCH 1/6] error: Trim includes after "Move qemu_error & friends..."
Luiz Capitulino writes:
> On Fri, 19 Mar 2010 22:31:05 +0100
> Markus Armbruster wrote:
>
>> Luiz Capitulino writes:
>>
>> > On Wed, 17 Mar 2010 18:56:49 +0100
>> > Markus Armbruster wrote:
>> >
>> >> Missed in commit 2f792016.
>> >>
>> >> Signed-off-by: Markus Armbruster
>> >> ---
>> >> hw/qdev-properties.c |1 +
>> >> monitor.c|2 --
>> >> sysemu.h |2 --
>> >> 3 files changed, 1 insertions(+), 4 deletions(-)
>> >>
>> >> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
>> >> index 92d6793..157a111 100644
>> >> --- a/hw/qdev-properties.c
>> >> +++ b/hw/qdev-properties.c
>> >> @@ -1,6 +1,7 @@
>> >> #include "sysemu.h"
>> >> #include "net.h"
>> >> #include "qdev.h"
>> >> +#include "qerror.h"
>> >>
>> >> void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
>> >> {
>> >> diff --git a/monitor.c b/monitor.c
>> >> index 0448a70..822dc27 100644
>> >> --- a/monitor.c
>> >> +++ b/monitor.c
>> >> @@ -49,10 +49,8 @@
>> >> #include "qint.h"
>> >> #include "qfloat.h"
>> >> #include "qlist.h"
>> >> -#include "qdict.h"
>> >> #include "qbool.h"
>> >> #include "qstring.h"
>> >> -#include "qerror.h"
>> >> #include "qjson.h"
>> >> #include "json-streamer.h"
>> >> #include "json-parser.h"
>> >> diff --git a/sysemu.h b/sysemu.h
>> >> index 8a9c630..9d3d51d 100644
>> >
>> > Hmm, why this second hunk? Also note that we have qemu-objects.h.
>>
>> The one below or the one above?
>
> The one above, but I believe it's because monitor.h includes them,
> right? This is fine.
Right.
[...]
Re: [Qemu-devel] Build always fail on x86_32 host for i386_softmmu target
On Mon, Mar 22, 2010 at 09:31:09AM +0100, Stefan Weil wrote:
> Bruce Majia schrieb:
> > Hi,
> >
> > When I built qemu on my x86_32 host with following configure line:
> >
> > $ ./configure --prefix=/usr/local/qemus/master \
> > --target-list=i386-softmmu
> > $ make
> >
> > The build will always fail with message:
> >
> > ...
> > CC i386-softmmu/fpu/softfloat-native.o
> > /mnt/farm/my_repo/qemu/fpu/softfloat-native.c:130:5: error:
> > "HOST_LONG_BITS" is not defined
> > make[1]: *** [fpu/softfloat-native.o] Error 1
> > make: *** [subdir-i386-softmmu] Error 2
> >
> >
> > Is this a known issue or something wrong with my configure line?
> >
> > Though I can make it work with a minor nasty patch:
> > ==
> > diff --git a/fpu/softfloat-native.c b/fpu/softfloat-native.c
> > index 049c830..5ba5013 100644
> > --- a/fpu/softfloat-native.c
> > +++ b/fpu/softfloat-native.c
> > @@ -127,6 +127,9 @@ floatx80 int64_to_floatx80( int64_t v STATUS_PARAM)
> > #endif
> >
> > /* XXX: this code implements the x86 behaviour, not the IEEE one. */
> > +#ifndef HOST_LONG_BITS
> > +#define HOST_LONG_BITS 32
> > +#endif
> > #if HOST_LONG_BITS == 32
> > static inline int long_to_int32(long a)
> > {
> > ==
> >
> > I thought it may necessary to ask if something wrong with above hack.
> > And can we get the problem fixed properly?
> >
> > Thanks.
> > -b
>
> I had this problem with incremental builds several times, too.
> A new make from scratch (all generated files removed) always worked,
> so I don't think your patch is needed.
Yes, you are right. I must messed up several incremental builds and
clean build. While I just tried clean configure and build, I just got
warning like this:
fpu/softfloat-native.c:132:5: warning: "HOST_LONG_BITS" is not defined
That is fine to me. Though it would be better to avoid the warning. :)
Thanks.
-b
Re: [Qemu-devel] Re: >2 serial ports?
Paul Brook wrote at Wed, 17 Mar 2010 11:18:09 +:
>> Oh, well, yes, I remember. qemu is more strict on ISA irq sharing now.
>> A bit too strict.
>>
>> /me goes dig out a old patch which never made it upstream for some
>> reason I forgot. Attached.
>
> This is wrong. Two devices should never be manipulating the same qemu_irq
> object. If you want multiple devices connected to the same IRQ then you need
> an explicit multiplexer. e.g. arm_timer.c:sp804_set_irq.
So... what we have to do here?
I've looked at the mentioned routine, here it is:
/* Merge the IRQs from the two component devices. */
static void sp804_set_irq(void *opaque, int irq, int level)
{
sp804_state *s = (sp804_state *)opaque;
s->level[irq] = level;
qemu_set_irq(s->irq, s->level[0] || s->level[1]);
}
But I know nothing about qemu internals, so don't quite
understand how to do this in case of serial ports. I
see it is tracking two timers and raises the irq level
if at least one half is raised... That to say - I've
got the idea, but how to apply it to serial ports?
Thanks!
/mjt
Re: [Qemu-devel] Build always fail on x86_32 host for i386_softmmu target
Bruce Majia schrieb:
> Hi,
>
> When I built qemu on my x86_32 host with following configure line:
>
> $ ./configure --prefix=/usr/local/qemus/master \
> --target-list=i386-softmmu
> $ make
>
> The build will always fail with message:
>
> ...
> CC i386-softmmu/fpu/softfloat-native.o
> /mnt/farm/my_repo/qemu/fpu/softfloat-native.c:130:5: error:
> "HOST_LONG_BITS" is not defined
> make[1]: *** [fpu/softfloat-native.o] Error 1
> make: *** [subdir-i386-softmmu] Error 2
>
>
> Is this a known issue or something wrong with my configure line?
>
> Though I can make it work with a minor nasty patch:
> ==
> diff --git a/fpu/softfloat-native.c b/fpu/softfloat-native.c
> index 049c830..5ba5013 100644
> --- a/fpu/softfloat-native.c
> +++ b/fpu/softfloat-native.c
> @@ -127,6 +127,9 @@ floatx80 int64_to_floatx80( int64_t v STATUS_PARAM)
> #endif
>
> /* XXX: this code implements the x86 behaviour, not the IEEE one. */
> +#ifndef HOST_LONG_BITS
> +#define HOST_LONG_BITS 32
> +#endif
> #if HOST_LONG_BITS == 32
> static inline int long_to_int32(long a)
> {
> ==
>
> I thought it may necessary to ask if something wrong with above hack.
> And can we get the problem fixed properly?
>
> Thanks.
> -b
I had this problem with incremental builds several times, too.
A new make from scratch (all generated files removed) always worked,
so I don't think your patch is needed.
Regards,
Stefan
Re: [Qemu-devel] virtio block device and sysfs
Jamie Lokier wrote: Anthony Liguori wrote: On 03/06/2010 04:42 PM, Marc Haber wrote: Hi, I am looking to get in touch with somebody who knows more about the connection between host configuration, qemu, kvm, and the virtio block device driver guest side than I know. My goal is to have a possibility to give a "speaking" name to any block device handed into a guest instance by the host. That name should be visible inside the guest, just as a LV is visible with its name in the system running the LVM. For example I would like to say on the qemu or kvm command line '-drive file=some-file,label=some-label,if=virtio', and have the string "some-label" show up somewhere in /sys/block in the guest, much as /sys/block/sda/device/model shows the hardware vendor and type for a standard SATA disk. The guest could then handle the information passed into it by the host with udev rules, allowing fstab constructs like "mount /dev/virtio/block/by-label/some-label as /usr" Sorry, I missed Anthony's earlier mail. You probably would just want to plumb ,serial=X into the virtio-blk config space and have the driver use it. Then you can do /dev/block/by-id/X This is a tad ironic as that is how this saga begun. Namely stuffing 20 bytes of serial number string into the virtio-blk PCI config space on qemu's side and pushing it over to the guest driver. I exposed this to the guest app via a new ioctl cmd which itself was the original point of contention. Someone took issue with introducing a new interface citing the existence of ATA and SCSI counterparts. However dragging in the associated baggage in order to emulate those interfaces unintentionally bloated usage of the config space to the point of breakage. To address this I'd moved from using config space to an unused BAR which (understandably) didn't go over too well. Somewhere along the line Rusty posted a minimal alternative version which directly used a virtio request to retrieve the data from qemu which is arguably the right way to do the job. That said we still had a dispute over what interface would be used to pass the S/N back to the guest: a new interface or reuse of an existing interface (eg: ATA IDENTIFY). That's where things fizzled when we couldn't immediately resolve the issue. So publishing the S/N in /sys would seem to side step this snag. I could have swore I sent out a guest-driver-app-interface-less version of the patch using virtio to pass the S/N but didn't find it in the archives. I did however locate it and can bring it forward as a reference for the above if interest exists. -john -- john.coo...@third-harmonic.com
