Re: [Qemu-devel] [PATCH 1/3] Target-microblaze: Remove unnecessary variable
Michael Tokarev writes: > 25.09.2015 11:37, Shraddha Barke wrote: >> Compress lines and remove the variable . > > Applied to -trivial, removing this piece of commit message: > > --- >> Change made using Coccinelle script >> >> @@ >> expression ret; >> @@ >> - if (ret) return ret; >> - return 0; >> + return ret; >> @@ >> local idexpression ret; >> expression e; >> @@ >> - ret = e; >> - return ret; >> + return e; >> @@ >> type T; identifier i; >> @@ >> - T i; >> ... when != i > --- Why? I like having the semantic patch in the commit message when there's any chance we'll want do the same mechanical change again later. You could save space and include it by reference, though: "Same Coccinelle semantic patch as is commit 74c373e". -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] Announcing qboot, a minimal x86 firmware for QEMU
Peter Maydell writes: > On 22 May 2015 at 12:12, Daniel P. Berrange wrote: >> Yep, it is hard saying no - but I'd think as long as it was possible to add >> the extra features using -device, it ought to be practical to keep a "virt" >> machine types "-nodefaults -nodefconfig" base setup pretty minimal. > > Mmm, but -device only works for pluggable devices really. We don't > have a coherent mechanism for saying "put the PS/2 keyboard controller > into the system at its usual IO ports" on the command line. ... yet. The qdev long-term vision has always been to support starting with an empty board, then add device models and their wiring. Unfortunately, progress towards that goal has been slow. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [BUG] Balloon malfunctions with memory hotplug
Luiz Capitulino writes: > Hello, > > Reproducer: > > 1. Start QEMU with balloon and memory hotplug support: > > # qemu [...] -m 1G,slots=2,maxmem=2G -balloon virtio > > 2. Check balloon size: > > (qemu) info balloon > balloon: actual=1024 > (qemu) > > 3. Hotplug some memory: > > (qemu) object_add memory-backend-ram,id=mem1,size=1G > (qemu) device_add pc-dimm,id=dimm1,memdev=mem1 > > 4. This is step is _not_ needed to reproduce the problem, >but you may need to online memory manually on Linux so >that it becomes available in the guest > > 5. Check balloon size again: > > (qemu) info balloon > balloon: actual=1024 > (qemu) > > BUG: The guest now has 2GB of memory, but the balloon thinks > the guest has 1GB Impact other than "info balloon"? > One may think that the problem is that the balloon driver is > ignoring hotplugged memory. This is not what's happening. If > you do balloon your guest, there's nothing stopping the > balloon driver in the guest from ballooning hotplugged memory. > > The problem is that the balloon device in QEMU needs to know > the current amount of memory available to the guest. > > Before memory hotplug this information was easy to obtain: the > current amount of memory available to the guest is the memory the > guest was booted with. This value is stored in the ram_size global > variable in QEMU and this is what the balloon device emulation > code uses today. However, when memory is hotplugged ram_size is > _not_ updated and the balloon device breaks. > > I see two possible solutions for this problem: > > 1. In addition to reading ram_size, the balloon device in QEMU >could scan pc-dimm devices to account for hotplugged memory. > >This solution was already implemented by zhanghailiang: > > http://lists.gnu.org/archive/html/qemu-devel/2014-11/msg02362.html > >It works, except that on Linux memory hotplug is a two-step >procedure: first memory is inserted then it has to be onlined >from user-space. So, if memory is inserted but not onlined >this solution gives the opposite problem: the balloon device >will report a larger memory amount than the guest actually has. > >Can we live with that? I guess not, but I'm open for discussion. > >If QEMU could be notified when Linux makes memory online, then >the problem would be gone. But I guess this can't be done. > > 2. Modify the balloon driver in the guest to inform the balloon >device on the host about the current memory available to the >guest. This way, whenever the balloon device in QEMU needs >to know the current amount of memory in the guest, it asks >the guest. This drops any usage of ram_size in the balloon >device What happens when the guest lies? >I'm not completely sure this is feasible though. For example, >what happens if the guest reports a memory amount to QEMU and >right after this more memory is plugged? > >Besides, this solution is more complex than solution 1 and >won't address older guests. > > Another important detail is that, I *suspect* that a very similar > bug already exists with 32-bit guests even without memory > hotplug: what happens if you assign 6GB to a 32-bit without PAE > support? I think the same problem we're seeing with memory > hotplug will happen and solution 1 won't fix this, although > no one seems to care about 32-bit guests... Fun... -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: encryption
Stefan Hajnoczi writes: > On Mon, Feb 16, 2015 at 06:19:04PM +0100, Henry Noack wrote: >> it is possible to decrypt a kvm volume only by using the command line after >> starting it? > > Encryption can be done at 3 levels: [...] > 2. In QEMU with qcow2, although this feature is not widely used and not >up to modern disk encryption standards. Quoting the fine manual: The use of encryption in qcow and qcow2 images is considered to be flawed by modern cryptography standards, suffering from a number of design problems: − The AES-CBC cipher is used with predictable initialization vectors based on the sector number. This makes it vulnerable to chosen plaintext attacks which can reveal the existence of encrypted data. − The user passphrase is directly used as the encryption key. A poorly chosen or short passphrase will compromise the security of the encryption. − In the event of the passphrase being compromised there is no way to change the passphrase to protect data in any qcow images. The files must be cloned, using a different encryption passphrase in the new file. The original file must then be securely erased using a program like shred, though even this is ineffective with many modern storage technologies. Use of qcow / qcow2 encryption is thus strongly discouraged. Users are recommended to use an alternative encryption technology such as the Linux dm-crypt / LUKS system. [...] -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call for agend for 2014-09-16
Juan Quintela writes: > Hi > > Please, send any topic that you are interested in covering. > > People have complained on the past that I don't cancel the call until > the very last minute. So, what do you think that deadline for > submitting topics is 23:00UTC on Monday? Yes, please. [...] -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v2 1/2] contrib: add ivshmem client and server
David Marchand writes: > When using ivshmem devices, notifications between guests can be sent as > interrupts using a ivshmem-server (typical use described in documentation). > The client is provided as a debug tool. [...] > diff --git a/contrib/ivshmem-client/ivshmem-client.c > b/contrib/ivshmem-client/ivshmem-client.c > new file mode 100644 > index 000..32ef3ef > --- /dev/null > +++ b/contrib/ivshmem-client/ivshmem-client.c > @@ -0,0 +1,418 @@ > +/* > + * Copyright(c) 2014 6WIND S.A. > + * All rights reserved. > + * > + * This work is licensed under the terms of the GNU GPL, version 2. See > + * the COPYING file in the top-level directory. > + */ Do you have a compelling reason why you can't license under GPLv2+? If yes, please explain it to us. If no, please use * This work is licensed under the terms of the GNU GPL, version 2 or * later. See the COPYING file in the top-level directory. [...] -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: KVM call for 2014-07-08
Markus Armbruster writes: > Please send topics. No topics, no call today. Happy hacking! [...] -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
KVM call for 2014-07-08
Please send topics. Call is on every other Tuesday at 15:00 CEST 13:00 UTC 09:00 EDT If you need phone number details, contact me privately. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] Why I advise against using ivshmem
Stefan Hajnoczi writes: > On Tue, Jun 17, 2014 at 11:44:11AM +0200, Paolo Bonzini wrote: >> Il 17/06/2014 11:03, David Marchand ha scritto: >> >>Unless someone steps up and maintains ivshmem, I think it should be >> >>deprecated and dropped from QEMU. >> > >> >Then I can maintain ivshmem for QEMU. >> >If this is ok, I will send a patch for MAINTAINERS file. >> >> Typically, adding yourself to maintainers is done only after having proved >> your ability to be a maintainer. :) >> >> So, let's stop talking and go back to code! You can start doing what was >> suggested elsewhere in the thread: get the server and uio driver merged into >> the QEMU tree, document the protocol in docs/specs/ivshmem_device_spec.txt, >> and start fixing bugs such as the ones that Markus reported. > > One more thing to add to the list: > > static void ivshmem_read(void *opaque, const uint8_t * buf, int flags) > > The "flags" argument should be "size". Size should be checked before > accessing buf. > > Please also see the bug fixes in the following unapplied patch: > "[PATCH] ivshmem: fix potential OOB r/w access (#2)" by Sebastian Krahmer > https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg03538.html Another one: most devices can be controlled via a dedicated CONFIG_, but not ivshmem: it uses CONFIG_KVM and CONFIG_PCI. Giving it its own CONFIG_IVSHMEM would be nice. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] Why I advise against using ivshmem
Some dropped quoted text restored. Vincent JARDIN writes: > Markus, > > see inline (I am not on all mailing list, please, keep the cc list). > >> Sure! The reasons for my dislike range from practical to >> philosophical. >> >> My practical concerns include: >> >> 1. ivshmem code needs work, but has no maintainer > See David's contributions: > http://patchwork.ozlabs.org/patch/358750/ We're grateful for David's patch for qemu-char.c, but this isn't ivshmem maintenance, yet. >> - Error handling is generally poor. For instance, "device_add >> ivshmem" kills your guest instantly. >> >> - More subjectively, I don't trust the code to be robust against >> abuse by our own guest, or the other guests sharing the memory. >> Convincing me would take a code audit. >> >> - MAINTAINERS doesn't cover ivshmem.c. >> >> - The last non-trivial commit that isn't obviously part of some >> tree-wide infrastructure or cleanup work is from September 2012 >> (commit c08ba66). >> >> 2. There is no libvirt support > > One can use qemu without libvivrt. You asked me for my reasons for disliking ivshmem. This is one. Sure, I can drink my water through a straw while standing on one foot, but that doesn't mean I have to like it. And me not liking it doesn't mean the next guy shouldn't like it. To each their own. >> 3. Out-of-tree server program required for full functionality >> >> Interrupts require a "shared memory server" running in the host (see >> docs/specs/ivshmem_device_spec.txt). It doesn't tell where to find >> one. The initial commit 6cbf4c8 points to >> . That repository's last commit is from >> September 2012. He's dead, Jim. >> >> ivshmem_device_spec.txt is silent on what the server is supposed to >> do. > > We have the source code, it provides the documentation to write our > own better server program. Good for you. Not good enough for the QEMU community. QEMU features requiring on out-of-tree software to be useful are fine, as long as said out-of-tree software is readily available to QEMU developers and users. Free software with a community around it and packaged in major distros qualifies. If you haven't got that, talk to us to find out whether what you've got qualifies, and if not, what you'd have to do to make it qualify. Back when we accepted ivshmem, the out-of-tree parts it needs were well below the "community & packaged" bar. But folks interested in it talked to us, and the fact that it's in shows that QEMU maintainers decided what they had then was enough. Unfortunately, we now have considerably less: Nahanni appears to be dead. An apparently dead git repository you can study is not enough. The fact that you hold an improved reimplementation privately is immaterial. So is the (plausible) claim that others could also create a reimplementation. >> If this server requires privileges: I don't trust it without an >> audit. >> >> 4. Out-of-tree kernel uio driver required > > No, it is optional. Good to know. Would you be willing to send a patch to ivshmem_device_spec.txt clarifying that? >> The device is "intended to be used with the provided UIO driver" >> (ivshmem_device_spec.txt again). As far as I can tell, the "provided >> UIO driver" is the one in the dead Nahanni repo. >> >> By now, you should be expecting this: I don't trust that one either. >> >> These concerns are all fixable, but it'll take serious work, and time. >> Something like: >> >> * Find a maintainer for the device model > I guess, we can find it into the DPDK.org community. >> * Review and fix its code >> >> * Get the required kernel module upstream > > which module? uio, it is not required. > >> * Get all the required parts outside QEMU packaged in major distros, or >>absorbed into QEMU > > Redhat did disable it. why? it is there in QEMU. Up to now, I've been wearing my QEMU hat. Let me exchange it for my Red one for a bit. We (Red Hat) don't just package & ship metric tons of random free software. We package & ship useful free software we can support for many, many years. Sometimes, we find that we have to focus serious development resources on making something useful supportable (Paolo mentioned qcow2). We obviously can't focus on everything, though. Anyway, ivshmem didn't make the cut for RHEL-7.0. Sorry if that inconveniences you. To get it into RHEL, you need to show it's both useful and supportable. Building a community around it would go a long way towards that. If you want to discuss this in more detail with us, you may want to try communication channels provided by your RHEL subscription in addition to the QEMU development mailing list. Don't be shy, you're paying for it! As always, I'm not speaking for myself, not my employer. Okay, wearing my QEMU hat again. >> In short, create a viable community around ivshmem, either within the >> QEMU community, or separately but cooperating. > > At least, DPDK.org community is a community using
Why I advise against using ivshmem (was: [Qemu-devel] Using virtio for inter-VM communication)
Henning Schild writes: > On Thu, 12 Jun 2014 08:48:04 +0200 > Markus Armbruster wrote: > >> Vincent JARDIN writes: >> >> > On 10/06/2014 18:48, Henning Schild wrote:> Hi, >> >> In a first prototype i implemented a ivshmem[2] device for the >> >> hypervisor. That way we can share memory between virtual machines. >> >> Ivshmem is nice and simple but does not seem to be used anymore. >> >> And it >> >> does not define higher level devices, like a console. >> > >> > FYI, ivhsmem is used here: >> > http://dpdk.org/browse/memnic/tree/ >> > >> > http://dpdk.org/browse/memnic/tree/pmd/pmd_memnic.c#n449 >> > >> > There are some few other references too, if needed. >> >> It may be used, but that doesn't mean it's maintained, or robust >> against abuse. My advice is to steer clear of it. > > Could you elaborate on why you advice against it? Sure! The reasons for my dislike range from practical to philosophical. My practical concerns include: 1. ivshmem code needs work, but has no maintainer - Error handling is generally poor. For instance, "device_add ivshmem" kills your guest instantly. - More subjectively, I don't trust the code to be robust against abuse by our own guest, or the other guests sharing the memory. Convincing me would take a code audit. - MAINTAINERS doesn't cover ivshmem.c. - The last non-trivial commit that isn't obviously part of some tree-wide infrastructure or cleanup work is from September 2012 (commit c08ba66). 2. There is no libvirt support 3. Out-of-tree server program required for full functionality Interrupts require a "shared memory server" running in the host (see docs/specs/ivshmem_device_spec.txt). It doesn't tell where to find one. The initial commit 6cbf4c8 points to . That repository's last commit is from September 2012. He's dead, Jim. ivshmem_device_spec.txt is silent on what the server is supposed to do. If this server requires privileges: I don't trust it without an audit. 4. Out-of-tree kernel uio driver required The device is "intended to be used with the provided UIO driver" (ivshmem_device_spec.txt again). As far as I can tell, the "provided UIO driver" is the one in the dead Nahanni repo. By now, you should be expecting this: I don't trust that one either. These concerns are all fixable, but it'll take serious work, and time. Something like: * Find a maintainer for the device model * Review and fix its code * Get the required kernel module upstream * Get all the required parts outside QEMU packaged in major distros, or absorbed into QEMU In short, create a viable community around ivshmem, either within the QEMU community, or separately but cooperating. On to the more philosophical ones. 5. Out-of-tree interface required Paraphrasing an old quip: Some people, when confronted with a problem, think "I know, I'll use shared memory." Now they have two problems. Shared memory is not an interface. It's at best something you can use to build an interface. I'd rather have us offer something with a little bit more structure. Very fast guest-to-guest networking perhaps. 6. Device models belong into QEMU Say you build an actual interface on top of ivshmem. Then ivshmem in QEMU together with the supporting host code outside QEMU (see 3.) and the lower layer of the code using it in guests (kernel + user space) provide something that to me very much looks like a device model. Device models belong into QEMU. It's what QEMU does. To all currently using ivshmem or contemplating its use: I'd like to invite you to work with the QEMU community to get your use case served better. You could start worse than with explaining it to us. In case you'd rather not work with the QEMU community: I'm not passing judgement on that (heck, I have had days when I'd rather not, too). But if somebody's reasons not to work with us include GPL circumvention, then that somebody is a scoundrel. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] Using virtio for inter-VM communication
Henning Schild writes: > On Thu, 12 Jun 2014 08:48:04 +0200 > Markus Armbruster wrote: > >> Vincent JARDIN writes: >> >> > On 10/06/2014 18:48, Henning Schild wrote:> Hi, >> >> In a first prototype i implemented a ivshmem[2] device for the >> >> hypervisor. That way we can share memory between virtual machines. >> >> Ivshmem is nice and simple but does not seem to be used anymore. >> >> And it >> >> does not define higher level devices, like a console. >> > >> > FYI, ivhsmem is used here: >> > http://dpdk.org/browse/memnic/tree/ >> > >> > http://dpdk.org/browse/memnic/tree/pmd/pmd_memnic.c#n449 >> > >> > There are some few other references too, if needed. >> >> It may be used, but that doesn't mean it's maintained, or robust >> against abuse. My advice is to steer clear of it. > > Could you elaborate on why you advice against it? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Using virtio for inter-VM communication
Vincent JARDIN writes: > On 10/06/2014 18:48, Henning Schild wrote:> Hi, >> In a first prototype i implemented a ivshmem[2] device for the >> hypervisor. That way we can share memory between virtual machines. >> Ivshmem is nice and simple but does not seem to be used anymore. >> And it >> does not define higher level devices, like a console. > > FYI, ivhsmem is used here: > http://dpdk.org/browse/memnic/tree/ > > http://dpdk.org/browse/memnic/tree/pmd/pmd_memnic.c#n449 > > There are some few other references too, if needed. It may be used, but that doesn't mean it's maintained, or robust against abuse. My advice is to steer clear of it. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
KVM call minutes for 2014-05-13
Today's agenda: - QOMifying both Memory regions and GPIOs and attaching them via QOM links (Peter Crosthwaite) Here are my notes; please correct mistakes. Peter C: Everything with a connection becomes a QOM object, references become links Peter Maydell: Okay PC: Don't include address spaces in v1, just memory regions and GPIO PM: Okay What's the root memory region's canonical path? PM: There is no root What about the big memory region in exec.c? PM: Should go away eventually Andreas Färber: Converting to types is a no-brainer, but property names and such are ABI, need to be very careful What's the connection to Edgar's CPU work? AF: Not everyone one the call is a memory API expert, please explain address space and memory regions PM: Address space is flattended memory regions, needed for actual memory access when constructing machines, we deal with memory regions, not address spaces AF+PM: Each CPU should have its own address space Does this duplicate address spaces? PM: Yes, but it's okay, because (a) they don't change very often (b) if performance turns out to suffer, we can cache or share AF: How much memory do address spaces use? PM: Don't know, guess not much Do busmasters get their own address space, too? PM: Yes Back to canonical paths Can we get them right? [I missed details here] Where to start? PC[?] suggests softip [I almost certainly got this name wrong] has CPUs with private memory, good example for separate address spaces How do separate address spaces affect gdbstub? threads can have their own view of memory already, we should be fine AF: Does per-CPU address space imply per-CPU memory regions? PM: Not necessarily, depends on board Back to canonical path of memory region AF: the memory region object needs to go *somewhere*! possible solution: if board code doesn't put the global memory region anywhere, put it in machine/unattached When does a QOM object need a path? AF: it should always have a path, but it really needs one when it's referenced via QMP or by a link General direction seems uncontroversial PC intends to post patches soon How is this work connected to vfio hotplug, and how to move best vfio hotplug forward? Alex Graf intends to respin his platform device patches -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call agenda for 2014-04-28
"Daniel P. Berrange" writes: > On Tue, Apr 29, 2014 at 02:33:58PM +0200, Markus Armbruster wrote: >> Peter Maydell writes: >> >> > On 29 April 2014 11:09, Michael S. Tsirkin wrote: >> >> Let's just make clear how to contact us securely, when to contact that >> >> list, and what we'll do with the info. I cobbled together the >> >> following: >> >> http://wiki.qemu.org/SecurityProcess >> > >> > Looks generally OK I guess. I'd drop the 'how to use pgp' section -- >> > anybody who cares will already know how to send us PGP email. >> >> The first paragraph under "How to Contact Us Securely" is fine, the rest >> seems redundant for readers familiar with PGP, yet hardly sufficient for >> the rest. >> >> One thing I like about Libvirt's Security Process page[*] is they give >> an idea on embargo duration. > > FWIW I picked the "2 weeks" length myself a completely arbitrary timeframe. > We haven't stuck to that strictly - we consider needs of each vulnerability > as it is triaged to determine the minimum practical embargo time. So think > of "2 weeks" as more of a guiding principal to show the world that we don't > believe in keeping issues under embargo for very long periods of time. Pretty much the way I read it :) The point I care about is a commitment to getting fixes out quickly, making clear we're not going to abuse "responsible disclosure" to cover dragging of feet and deflecting blame. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call agenda for 2014-04-28
Peter Maydell writes: > On 29 April 2014 11:09, Michael S. Tsirkin wrote: >> Let's just make clear how to contact us securely, when to contact that >> list, and what we'll do with the info. I cobbled together the >> following: >> http://wiki.qemu.org/SecurityProcess > > Looks generally OK I guess. I'd drop the 'how to use pgp' section -- > anybody who cares will already know how to send us PGP email. The first paragraph under "How to Contact Us Securely" is fine, the rest seems redundant for readers familiar with PGP, yet hardly sufficient for the rest. One thing I like about Libvirt's Security Process page[*] is they give an idea on embargo duration. [*] http://libvirt.org/securityprocess.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call agenda for 2014-04-28
Juan Quintela writes: > Hi > > Please, send any topic that you are interested in covering. [...] I'd like to have these things settled sooner than five minutes before the scheduled hour, so here goes: call or no call? Agenda? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] Who signed gemu-1.7.1.tar.bz2?
Anthony Liguori writes: > On 04/22/14 07:35, Michael Roth wrote: >> Quoting Stefan Hajnoczi (2014-04-22 08:31:08) >>> On Wed, Apr 02, 2014 at 05:40:23PM -0700, Alex Davis wrote: and where is their gpg key? >>> >>> Michael Roth is doing releases: >>> >>> http://pgp.mit.edu/pks/lookup?op=vindex&search=0x3353C9CEF108B584 >>> >>> >>> > $ gpg --verify qemu-2.0.0.tar.bz2.sig >>> gpg: Signature made Thu 17 Apr 2014 03:49:55 PM CEST using RSA >>> key ID F108B584 gpg: Good signature from "Michael Roth >>> " gpg: aka "Michael Roth >>> " gpg: aka "Michael Roth >>> " >> >> Missed the context, but if this is specifically about 1.7.1: >> >> 1.7.1 was prior to me handling the release tarballs, Anthony >> actually did the signing and uploading for that one. I'm a bit >> confused though, as the key ID on that tarball is: >> >> mdroth@loki:~/Downloads$ gpg --verify qemu-1.7.1.tar.bz2.sig gpg: >> Signature made Tue 25 Mar 2014 09:03:24 AM CDT using RSA key ID >> ADF0D2D9 gpg: Can't check signature: public key not found >> >> I can't seem to locate ADF0D2D9 though: >> >> http://pgp.mit.edu/pks/lookup?search=0xADF0D2D9&op=vindex >> >> Anthony's normal key (for 1.6.0 and 1.7.0 at least) was 7C18C076: >> >> http://pgp.mit.edu/pks/lookup?search=0x7C18C076&op=vindex >> >> I think maybe Anthony might've signed it with a separate local >> key? > > Yeah, I accidentally signed it with the wrong key. Replacing the > signature doesn't seem like the right thing to do since release > artifacts should never change. You could still publish the key, with some suitable signatures. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call agenda for 2014-04-15
Alexander Graf writes: >> Am 15.04.2014 um 18:56 schrieb Markus Armbruster : >> >> Alexander Graf writes: >> >>>> On 04/15/2014 04:00 PM, Markus Armbruster wrote: >>>> Juan Quintela writes: >>>> >>>>> Juan Quintela wrote: >>>>>> Hi >>>>>> >>>>>> Please, send any topic that you are interested in covering. >>>>>> >>>>>> Thanks, Juan. >>>>> As there are no topics, no call. >>>> Did we have a call anyway? IRC log looks like we did... >>> >>> Yes, we did. Whoever attended, please reply with things I mixed up or >>> forgot. >>> >>> Two topics: >> [...] >>> 2) -device for non-PCI devices >> >> Interesting stuff, hate to have missed it. >> >> Can we please ensure more useful advance notice than "monitor the IRC >> channel around call time"? Decide call / no call the night before would >> work nicely for me. > > I sent an email to the list earlier this werk as reply to the last > call, because there was no mail for this week's agenda yet. That's probably why Juan didn't pick it up. Explanation, not criticizing either of you. If we post the call's agenda the night before, there's time to correct omissions. I'm fine with late agenda changes. I really don't like last minute call / no call decisions. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call agenda for 2014-04-15
Alexander Graf writes: > On 04/15/2014 04:00 PM, Markus Armbruster wrote: >> Juan Quintela writes: >> >>> Juan Quintela wrote: >>>> Hi >>>> >>>> Please, send any topic that you are interested in covering. >>>> >>>> Thanks, Juan. >>> As there are no topics, no call. >> Did we have a call anyway? IRC log looks like we did... > > Yes, we did. Whoever attended, please reply with things I mixed up or > forgot. > > Two topics: [...] > 2) -device for non-PCI devices Interesting stuff, hate to have missed it. Can we please ensure more useful advance notice than "monitor the IRC channel around call time"? Decide call / no call the night before would work nicely for me. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call agenda for 2014-04-15
Juan Quintela writes: > Juan Quintela wrote: >> Hi >> >> Please, send any topic that you are interested in covering. >> >> Thanks, Juan. > > As there are no topics, no call. Did we have a call anyway? IRC log looks like we did... -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call agenfda for 2014-04-01
[Cc: Andreas, Anthony] Alexander Graf writes: > On 10.04.2014, at 17:52, Peter Maydell wrote: > >> On 10 April 2014 16:49, Alexander Graf wrote: >>> For the next call, I would propose to revive the "platform bus" >>> (aka: how to create non-PCI devices with -device) discussions Rather: devices that connect to more than just a bus. Since pseudo-bus sysbus provides exactly no connections, sysbus devices generally connect to more. Currently, the only way to make such additional connections is special-purpose code. -device's connection engine can only connect to a bus. >>> to make sure we're all on the same page. >> >> I rather suspect we are not :-) Do you have a link to >> the current proposals for prior reading? > > The only thing I could find is the old thread about my platform bus > approach (which Anthony disliked): > > https://lists.gnu.org/archive/html/qemu-devel/2013-07/msg03614.html > > So from what I remember the plan moving forward was to have a special > device type similar to my platform bus devices that you can just > create using -device, no bus involved. The machine file would then > loop through them, interpret the "I sit at address x" and "I want > interrupt number y" fields to link them to whatever the machine model > thinks is a good fit. > > The same way the machine model today has to have knowledge on each > device tree node type it generates, it would do the same for these > devices. So the machine has to have awareness of all the "funky > special options" a device tree node receives - the same as for any > other device. Just that in this case it wouldn't be able to hardcode > them, but have to generate them on the fly when it sees a device in > the object tree. The following is just my understanding of where we're trying to go. The people active in this field (Andreas?) should correct misunderstandings. Our overall goal is building machines from components by *data* rather than code. Once it's data, it can be made run-time configuration. The configuration describes how components are to be connected, and generic connection code makes the connections guided by the configuration. The current generic connection code can only connect a device to a single bus. If you don't specify the bus, it picks one. Which one it picks is effectively ABI. Picking a bus is a special case of "pick a connection if user didn't specify one". The current bus-picker doesn't depend on the machine type, but that's not going to hold in general. I like to think of the "pick connections the user didn't specify" engine as conceptually separate from the "make connections driven by configuration" engine. The actual code isn't so separate, but that's implementation. How does your "platform device" proposal (for lack of a better expression) fit into this general framework of ideas? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call agenda for 2014-04-01
Anthony Liguori writes: > On Mon, Mar 31, 2014 at 7:46 AM, Andreas Färber wrote: >> Am 31.03.2014 16:32, schrieb Peter Maydell: >>> On 31 March 2014 15:28, Paolo Bonzini wrote: I think it would be a good idea to separate the committer and release manager roles. Peter is providing the community with a wonderful service, just like you were; putting too much work on his shoulders risks getting us in the same situation if anything were to affect his ability to provide it. >>> >>> Yes, I strongly agree with this. I think we'll do much better >>> if we can manage to share out responsibilities among a wider >>> group of people. >> >> May I propose Michael Roth, who is already experienced from the N-1 >> stable releases? >> >> If we can enable him to upload the tarballs created from his tags that >> would also streamline the stable workflow while at it. > > If mdroth is willing to take this on, I am very supportive. Another vote of confidence from me. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] Massive read only kvm guests when backing file was missing
"Michael S. Tsirkin" writes: > On Wed, Mar 26, 2014 at 11:08:03PM -0300, Alejandro Comisario wrote: >> Hi List! >> Hope some one can help me, we had a big issue in our cloud the other >> day, a couple of our openstack regions ( +2000 kvm guests with qcow2 ) >> went read only filesystem from the guest side because the backing >> files directory (the openstack _base directory) was compromised and >> the data was lost, when we realized the data was lost, it took us 5 >> mins to restore the backup of the backing files, but by that time all >> the kvm guests received some kind of IO error from the hypervisor >> layer, and went read only on root filesystem. >> >> My question would be, is there a way to hold the IO operations against >> the backing files ( i thought that would be 99% READ operations ) for >> a little longer ( im asking this because i dont quite understand what >> is the process and when it raises the error ) in a case the backing >> files are missing (no IO possible) but is recoverable within minutes ? >> >> Any tip on how to achieve this if possible, or information about how >> backing files works on kvm, will be amazing. >> Waiting for feedback! >> >> kindest regards. >> Alejandro Comisario > > > I'm guessing this is what happened: guests timed out meanwhile. > You can increase the timeout within the guest: > echo 600 > /sys/block/sda/device/timeout > to timeout after 10 minutes. > > If you have installed qemu guest agent on your system, you can do this > from the host. Unfortunately by default it's memory can be pushed out to swap > and then on disk error access there might will fail :( > Maybe we should consider mlock on all its memory at least as an option. > > You could pause your guests, restart them after the issue is resolved, > and we could I guess add functionality to pause VM on disk errors > automatically. > Stefan? Would -drive rerror=stop do? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call agenda for 2013-12-24
Juan Quintela writes: > Hi > > First of all, poll told to move the call earlier. > > 9:00 EST (15:00 CET or 6:00 Pacific) ACK > Are we having a call on the 24th? Do we have any topics? Are enough > people not on vacation? I'm not not on vacation, and I hope y'all aren't not on vacation, either ;-) > > Happy Christmas Seconded! -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call agenda for 2013-12-10
Anthony Liguori writes: > On Tue, Dec 10, 2013 at 4:54 AM, Markus Armbruster wrote >> Paolo Bonzini writes: >> >>> Il 10/12/2013 12:42, Juan Quintela ha scritto: >>>> >>>> Hi >>>> >>>> Please, send any topic that you are interested in covering. >>> >>> May not need a phone call, but I'll drop it here: what happened to >>> acknowledgement emails from the patches script? >>> >>> Also, Anthony, it looks like you're still adjusting to the new job. If >>> you need help with anything, I guess today's call could be a good place >>> to discuss it. >>> >>> And someone needs to send out the email saying that 1.7.0 is out and >>> that the next version will be 2.0! >> >> Speaking of sending out e-mail: did I miss the promised followup to the >> key signing party? > > I need to find the papers from KVM Forum which are somewhere among the > stacks of boxes here :-/ Please do, because the longer the delay, the fewer participants will be willing and able to find *their* papers. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call agenda for 2013-12-10
Paolo Bonzini writes: > Il 10/12/2013 12:42, Juan Quintela ha scritto: >> >> Hi >> >> Please, send any topic that you are interested in covering. > > May not need a phone call, but I'll drop it here: what happened to > acknowledgement emails from the patches script? > > Also, Anthony, it looks like you're still adjusting to the new job. If > you need help with anything, I guess today's call could be a good place > to discuss it. > > And someone needs to send out the email saying that 1.7.0 is out and > that the next version will be 2.0! Speaking of sending out e-mail: did I miss the promised followup to the key signing party? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v2 0/2]
Your cover letter lacks a proper subject, and your patches lack proper threading. git-send-email threads properly (unless you mess up its configuration). -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: KVM Guest keymap issue
Not specific to KVM, adding qemu-devel. Matej Mailing writes: > Dear list, > > I have a problem with a Windows XP guest that I connect to via VNC and > is using "sl" keymap (option -k sl). > > The guest is Windows XP and the problematic characters are s, c and z > with caron... when I type them via VNC, they are not printed at all in > virtual system... I have checked the file /usr/share/kvm/keymaps/sl > and it seems that it contains different codes than I get when doing > showkey --ascii on the host machine (running Ubuntu 12.04). I have > tried to change the KVM's keymap file 'sl' with the codes I get from > showkey, but they are still not printed in virtual system to which I > am connected via VNC... > > I am totally lost with this issue, thanks for your time and ideas. Required reading for anyone struggling with virtual keyboards: https://www.berrange.com/posts/2010/07/04/more-than-you-or-i-ever-wanted-to-know-about-virtual-keyboard-handling/ -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [SeaBIOS] KVM call agenda for 2013-05-28
Anthony Liguori writes: > Gerd Hoffmann writes: > >> On 05/29/13 01:53, Kevin O'Connor wrote: >>> On Thu, May 23, 2013 at 03:41:32PM +0300, Michael S. Tsirkin wrote: Juan is not available now, and Anthony asked for agenda to be sent early. So here comes: Agenda for the meeting Tue, May 28: - Generating acpi tables >>> >>> I didn't see any meeting notes, but I thought it would be worthwhile >>> to summarize the call. This is from memory so correct me if I got >>> anything wrong. >>> >>> Anthony believes that the generation of ACPI tables is the task of the >>> firmware. Reasons cited include security implications of running more >>> code in qemu vs the guest context, >> >> I fail to see the security issues here. It's not like the apci table >> generation code operates on untrusted input from the guest ... > > But possibly untrusted input from a malicious user. You can imagine > something like a IaaS provider that let's a user input arbitrary values > for memory, number of nics, etc. > > It's a stretch of an example, I agree, but the general principle I think > is sound: we should push as much work as possible to the least > privileged part of the stack. In this case, firmware has much less > privileges than QEMU. Firmware runs in a primitive, unforgiving environment, and should do as little as humanly possible. For an instructive example of deviating from that rule, check out UEFI. [...] -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH V4 RESEND 15/22] tap: multiqueue support
Commit 264986e2 extended NetdevTapOptions without updating the
documentation. Hasn't been addressed since. Must fix for 1.4, in my
opinion.
This is the offending patch:
Jason Wang writes:
> Recently, linux support multiqueue tap which could let userspace call
> TUNSETIFF
> for a signle device many times to create multiple file descriptors as
> independent queues. User could also enable/disabe a specific queue through
> TUNSETQUEUE.
>
> The patch adds the generic infrastructure to create multiqueue taps. To
> achieve
> this a new parameter "queues" were introduced to specify how many queues were
> expected to be created for tap by qemu itself. Alternatively, management could
> also pass multiple pre-created tap file descriptors separated with ':'
> through a
> new parameter fds like -netdev tap,id=hn0,fds="X:Y:..:Z". Multiple vhost file
> descriptors could also be passed in this way.
>
> Each TAPState were still associated to a tap fd, which mean multiple TAPStates
> were created when user needs multiqueue taps. Since each TAPState contains one
> NetClientState, with the multiqueue nic support, an N peers of NetClientState
> were built up.
>
> A new parameter, mq_required were introduce in tap_open() to create multiqueue
> tap fds.
[...]
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 3a4817b..cdd8384 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2533,6 +2533,7 @@
>'data': {
> '*ifname': 'str',
> '*fd': 'str',
> +'*fds':'str',
> '*script': 'str',
> '*downscript': 'str',
> '*helper': 'str',
> @@ -2540,7 +2541,9 @@
> '*vnet_hdr': 'bool',
> '*vhost': 'bool',
> '*vhostfd':'str',
> -'*vhostforce': 'bool' } }
> +'*vhostfds': 'str',
> +'*vhostforce': 'bool',
> +'*queues': 'uint32'} }
>
> ##
> # @NetdevSocketOptions
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: I may have found a bug(unlikely) with Qemu-kvm 1.1
Veruca Salt writes: > We are upgrading, carefully, from 1.0 to 1.1. Keep going; we're about to release 1.4 ;) > We welcome the improved AC97 support and the loss of the ehci warning > message on startup. > > I am finding an issue getting through to the monitor however. > Neither ctrl-alt-shift-2 nor ctrl-alt-2 exposes the monitor. I never use that one, but it should work. > I have even tried using the -mon option code examples to force a > monitor to stdio, but the monitor stubbornly refuses to appear. > > Any advice would be greatly appreciated. These all work for me: * Shorthand syntax -monitor stdio * Equivalent longhand -chardev stdio,id=mon0 -mon chardev=mon0,mode=readline,default=on * Same as config file snippet, for use with -readconfig [chardev "mon0"] backend = "stdio" [mon] chardev = "mon0" mode = "readline" default = "on" -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] What to do about non-qdevified devices?
Andreas Färber writes: > Am 30.01.2013 13:35, schrieb Markus Armbruster: >> Peter Maydell writes: >> >>> On 30 January 2013 07:02, Markus Armbruster wrote: >>>> Anthony Liguori writes: >>>> >>>> [...] >>>>> The problems I ran into were (1) this is a lot of work (2) it basically >>>>> requires that all bus children have been qdev/QOM-ified. Even with >>>>> something like the ISA bus which is where I started, quite a few devices >>>>> were not qdevified still. >>>> >>>> So what's the plan to complete the qdevification job? Lay really low >>>> and quietly hope the problem goes away? We've tried that for about >>>> three years, doesn't seem to work. >>> >>> Do we have a list of not-yet-qdevified devices? Maybe we need to >>> start saying "fix X Y and Z or platform P is dropped from the next >>> release". (This would of course be easier if we had a way to let users >>> know that platform P was in danger...) >> >> I think that's a good idea. Only problem is identifying pre-qdev >> devices in the code requires code inspection (grep won't do, I'm >> afraid). > > +1 That would address my request as well. > > Having a list of low-hanging fruit on the Wiki might also give new > contributors some ideas of where and how to start poking at the code. > >> If we agree on a "qdevify or else" plan, I'd be prepared to help with >> the digging up of devices. > > I disagree on the "or else" part. I have been qdev'ifying and QOM'ifying > devices in my maintenance area, and progress is slow. It gets even Good work, much appreciated. > slower if one leaves clearly maintained areas. I see no good reason to > force a pistol on someone's breast, like you have done for IDE, unless > there is a good reason to do so. Currently I don't see any. There's the reason that made me hijack this thread. Paraphrashing Anthony: doing IRQs right involves Pin objects, and ultimately requires all bus children have been qdevified. Even for ISA, there are still stragglers holding us back. Is that sufficient reason to rip out devices *now*? No, and I didn't call for it. Could it become sufficient reason in the not too distant future? Possibly. Should we plan ahead for such a contingency? Probably. But I didn't call for that either. What I actually wrote was 1. I think mapping the remaining qdevification work is a good idea, and 2. if we commit to attempt doing that work in a reasonable time frame, I'd be willing to help with the mapping. Implying that without such a committment, sorry, got more immediately useful things to do. And by the way, the kind of "pistol" I get to brandish in this group is about as scary as a water pistol in the middle of the Gobi desert. [...] -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call minutes 2013-01-29 - Port I/O
Peter Maydell writes: > On 30 January 2013 11:39, Andreas Färber wrote: >> Proposal by hpoussin was to move _list_add() code to ISADevice: >> http://lists.gnu.org/archive/html/qemu-devel/2013-01/msg00508.html >> >> Concerns: >> * PCI devices (VGA, QXL) register I/O ports as well >> => above patches add dependency on ISABus to machines >> -> " no mac ever had one" >> => PCIDevice shouldn't use ISA API with NULL ISADevice >> * Lack of avi: Who decides about memory API these days? >> >> armbru and agraf concluded that moving this into ISA is wrong. >> >> => I will drop the remaining ioport patches from above series. >> >> Suggestions on how to proceed with tackling the issue are welcome. > > How does this stuff work on real hardware? I would have > expected that a PCI device registering the fact it has > IO ports would have to do so via the PCI controller it > is plugged into... > > My naive don't-know-much-about-portio suggestion is that this > should work the same way as memory regions: each device > provides portio regions, and the controller for the bus > (ISA or PCI) exposes those to the next layer up, and > something at board level maps it all into the right places. Makes sense me, but I'm naive, too :) For me, "I/O ports" are just an alternate address space some devices have. For instance, x86 CPUs have an extra pin for selecting I/O vs. memory address space. The ISA bus has separate read/write pins for memory and I/O. This isn't terribly special. Mapping address spaces around is what devices bridging buses do. I'd expect a system bus for an x86 CPU to have both a memory and an I/O address space. I'd expect an ISA PC's sysbus - ISA bridge to map both directly. I'd expect an ISA bridge for a sysbus without a separate I/O address space to map the ISA I/O address space into the sysbus's normal address space somehow. PCI ISA bridges have their own rules, but I've gotten away with ignoring the details so far :) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] What to do about non-qdevified devices?
Peter Maydell writes: > On 30 January 2013 07:02, Markus Armbruster wrote: >> Anthony Liguori writes: >> >> [...] >>> The problems I ran into were (1) this is a lot of work (2) it basically >>> requires that all bus children have been qdev/QOM-ified. Even with >>> something like the ISA bus which is where I started, quite a few devices >>> were not qdevified still. >> >> So what's the plan to complete the qdevification job? Lay really low >> and quietly hope the problem goes away? We've tried that for about >> three years, doesn't seem to work. > > Do we have a list of not-yet-qdevified devices? Maybe we need to > start saying "fix X Y and Z or platform P is dropped from the next > release". (This would of course be easier if we had a way to let users > know that platform P was in danger...) I think that's a good idea. Only problem is identifying pre-qdev devices in the code requires code inspection (grep won't do, I'm afraid). If we agree on a "qdevify or else" plan, I'd be prepared to help with the digging up of devices. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What to do about non-qdevified devices? (was: [Qemu-devel] KVM call minutes 2013-01-29)
Anthony Liguori writes: [...] > The problems I ran into were (1) this is a lot of work (2) it basically > requires that all bus children have been qdev/QOM-ified. Even with > something like the ISA bus which is where I started, quite a few devices > were not qdevified still. So what's the plan to complete the qdevification job? Lay really low and quietly hope the problem goes away? We've tried that for about three years, doesn't seem to work. [...] -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH] kvm: Set default accelerator to "kvm" if the host supports it
"Daniel P. Berrange" writes: > On Mon, Oct 01, 2012 at 06:43:00PM +0200, Andreas Färber wrote: >> Hello Jan, >> >> Am 01.10.2012 16:34, schrieb Jan Kiszka: >> > If we built a target for a host that supports KVM in principle, set the >> > default accelerator to KVM as well. This also means the start of QEMU >> > will fail to start if KVM support turns out to be unavailable at >> > runtime. >> >> From a distro point of view this of course means that we will build >> against KVM and that the new KVM default will start to fail for users on >> very old hardware. Can't we do a runtime check to select the default? > > NB, this is *not* only about old hardware. There are plenty of users who > use QEMU inside VMs. One very common usage I know of is image building > tools which are run inside Amazon VMs, using libguestfs & QEMU. > > IMHO, default to KVM, fallback to TCG is the most friendly default > behaviour. Friendly perhaps, generating an infinite series of questions "why is my guest slow as molasses?" certainly. And for each instance of the question, there's an unknown number of users who give QEMU a quick try, screw up KVM unknowingly, observe the glacial speed, and conclude it's crap. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call agenda for September 25th
Anthony Liguori writes: > Kevin Wolf writes: > >> Am 25.09.2012 14:57, schrieb Anthony Liguori: >>> Paolo Bonzini writes: >>> Il 24/09/2012 13:28, Juan Quintela ha scritto: > Hi > > Please send in any agenda items you are interested in covering. URI parsing library for glusterfs: libxml2 vs. in-tree "fork" of the same code. >>> >>> The call is a bit late for Bharata but I think copying is the way to go. >>> >>> Something I've been thinking about since this discussion started >>> though. Maybe we could standardize on using URIs as short-hand syntax >>> for backends. >> >> Compared with QemuOpts, it's not really short-hand or even convenient >> for manual use. For management tools it might be nice because URIs have >> a well-known syntax, can escape anything and implementations exist. But >> I think we must still maintain an easy to use syntax for human users. >> >>> For example: >>> >>> qemu -hda file:///foo.img >>> >>> Or: >>> >>> qemu -device virtio-net-pci,netdev=tap:///vnet0?script=/etc/qemu-ifup >>> >>> Or: >>> >>> qemu -device \ >>> isa-serial,index=0,chr=tcp://localhost:1025/?server=on&wait=off >> >> Your examples kind of prove this: They aren't much shorter than what >> exists today, but they contain ? and &, which are nasty characters on >> the command line. >> >>> This works particularly well with a "treat unknown options as -device" >>> mechanism so that we could do: >>> >>> qemu -isa-serial chr=tcp://localhost:1025/?server=on&wait=off >>> >>> We could even introduce a secondary implied option to shorten this >>> further to: >>> >>> qemu -isa-serial tcp://localhost:1025/?server=on&wait=off Too much magic for my taste. I'm afraid it leads to rather obscure error messages on misspellings. >> This is something that I was thinking of in the context of -blockdev a >> while ago (without URLs): Define the block device inside of -device >> specifications. The problem of nesting an option string inside another >> one is solved in theory by URLs because they allow (nested) escaping, >> but in practice we'll need to use some kind of brackets instead if we >> want it to be usable. > > qemu -isa-serial 'tcp://localhost:1025/?server=on&wait=off' > > I don't think it's really that better. And yeah, your thoughts are > exactly mine. Having two syntaxes allows us to use a single option. > > Hopefully most options could avoid having query parameters so escaping > wasn't a problem. It's unfortunate that the TCP character device uses > client mode by default. You could fold a limited set of common flags into the scheme. Prior art: socat supports syntax like TCP:: TCP4:: TCP-LISTEN: I'm not saying it's a good idea for QEMU. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: KVM Call minutes for 2012-09-25
Juan Quintela writes: > Hi > > This are this week minutes: > > - URI parsing library for glusterfs: libxml2 vs. in-tree "fork" of the > same code. (Paolo) > * code hasn't changed in 2 years, it is really stable > * anthony wants to copy the code > > - there are several commands that do blocking IO > dump-guest-memory/screen-dump > convert to asynchronous commands after we move all to QAPI > only two commands missingto port to QAPI, and one is posted on list > non-blocking IO to a file is a challenge > (we have code on the block layer for it) > > - how to give errors from OpenFile to the caller > putting errno as int: bad idea > putting as strerrno string: also a bad idea, no warantees Use the identifiers instead of their non-portable numeric encodings or strerror() descriptions: "EPERM", "EINVAL", ... > Using an extended error code > Put it as a string that can be parsed? > Paolo suggest to create a new error class > We don't warantee that errors would be "parseable". We don't want programs to parse the human-readable error string. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 4/4] kvm: i386: Add classic PCI device assignment
Anthony Liguori writes:
> Blue Swirl writes:
>
>> On Tue, Aug 28, 2012 at 5:28 PM, Michael S. Tsirkin wrote:
>>> On Tue, Aug 28, 2012 at 05:01:55PM +, Blue Swirl wrote:
On Tue, Aug 28, 2012 at 7:35 AM, Michael Tokarev wrote:
> On 27.08.2012 22:56, Blue Swirl wrote:
> []
>>> +static uint32_t slow_bar_readb(void *opaque, target_phys_addr_t addr)
>>> +{
>>> +AssignedDevRegion *d = opaque;
>>> +uint8_t *in = d->u.r_virtbase + addr;
>>
>> Don't perform arithmetic with void pointers.
>
> There are a few places in common qemu code which does this for a very
> long time. So I guess it is safe now.
It's a non-standard GCC extension.
>>>
>>> So? We use many other GCC extensions. grep for typeof.
>>
>> Dependencies should not be introduced trivially. In this case, it's
>> pretty easy to avoid void pointer arithmetic as Jan's next version
>> shows.
>
> The standard is vague with respect void arithmetic. Most compilers
> allow it. A very good analysis of the standard can be found below.
>
> http://stackoverflow.com/questions/3523145/pointer-arithmetic-for-void-pointer-in-c
>
> BTW: can we please stop arguing about C standards. If we currently are
> using something in QEMU that's supported by clang and GCC, it's fine and
> we ought to continue using it.
Seconded.
I don't see a point in making contributors avoid non-problems that might
conceivably become trivial problems some day. Especially when there's
no automated help with the avoiding.
[...]
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH uq/master] kvmvapic: Disable if there is insufficient memory
Jan Kiszka writes: > We need at least 1M of RAM to map the option ROM. Otherwise, we will > corrupt host memory or even crash. Let's put a reproducer in the commit message, if it's not too much trouble. Here's mine: $ qemu-system-x86_64 -nodefaults --enable-kvm -vnc :0 -m 640k Segmentation fault (core dumped) > Reported-by: Markus Armbruster > Signed-off-by: Jan Kiszka Tested-by: Markus Armbruster -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call minutes July 24th
Juan Quintela writes: > Hi > > Minutes of Today call. > > ahci: agraf > > - how to enable it? > * today is too difficult (alex) > * get it to work as everything else (markus) > And big discussion ensued. Markus & alex will reply with details. > No agreement, and too subtle for me to resume perfectly. > * cdrom still not working > - dates for freeze? q35? > depends on what people does/review? > > Later, Juan. Sorry for the delay. Hope I still remember enough write a worthwhile summary. Alex, please correct me if I misrepresent you. Background -- * -drive if=ide asks the board to connect this drive to its "IDE controller". It's silently ignored if the board doesn't have one. Even if it has one, it may silently ignore some drives. Examples: - PIIX3 boards such as current "pc" connect all if=ide drives to controller "piix3-ide". Bus numbers > 2 fail machine creation. - I expect the forthcoming "q35" board to connect all if=ide drives to controller "ich9-ahci". We hope to upgrade "pc" to this board. - Board "r2d" connects the if=ide,index=0 drive to a non-qdevified controller hw/ide/mmio.c. Any others get ignored. - Board "s390-virtio" connects some if=ide drives[*] to virtio-blk-s390. Yes, that's not really an IDE controller. - Board "highbank" appears to have an ich9-ahci controller, but it doesn't seem to connect any if=ide drives to it. * Likewise, -drive if=scsi asks the board to connect this drive to a SCSI controller. Examples: - PC boards create N+1 "lsi53c895a" controllers, where N is the largest bus number in any -drive if=scsi. The controller auto-connects all if=scsi,bus=I drives for its own bus number I. Ugly as hell. - Board "pseries" works the same, except it creates "spapr-vscsi" controllers. - The various Sun4[cdm] boards create a single "esp" controller, for bus=0. -drive if=scsi with any other bus fails machine initialization. * The meaning of -hda, drive without if=, and so forth depends on the board, it's currently either like if=scsi or like if=ide. * You can use -device to add additional controllers and drives. For instance, if the board provides a PCI bus, then -device ich9-ahci,id=ahci0 creates an ICH9 AHCI controller, and -drive if=none,id=drive0,... -device ide-hd,bus=ahci0.0,drive=drive0 connects an IDE disk to it. Alex's proposal --- Alex wants more users for AHCI. Using it with a board that doesn't connect if=ide drives to an AHCI controller involves -device. Alex thinks that's too hard for users, and prevents adoption. His proposed solution is to create a new interface type "ahci", so that -drive if=ahci "just works". It creates ich9-ahci controllers automatically. Critique * Does the use case "AHCI with a non-AHCI board" justify a new interface type? Would anybody care if we had q35 today? Would we regret adding if=ahci once we have q35? * If the board already has an ich9-ahci, you can't use if=ahci in its current form to add drives to it. if=ahci creates *additional* ich9-ahci controllers. * Do we want a separate interface type for each (sufficiently popular) controller? What about "megasas" when the board provides only "lsi53c895a"? What shall we do when we acquire an ich10 device model? Where does that stop? * Should -drive ever auto-create controllers? if=scsi does, but it's ugly. * A possible alternative: machine option to replace the controller to use for if=ide. Also possible for if=scsi. * Probably more I can't remember anymore. [*] "Some drives": looks like the code attempts to connect up to ten, but I'm not at all sure more than two work. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v3 00/16] net: hub-based networking
Anthony Liguori writes: > On 05/29/2012 04:14 PM, Markus Armbruster wrote: >> Luiz Capitulino writes: >> >>> On Mon, 28 May 2012 12:17:04 +0100 >>> Stefan Hajnoczi wrote: >>> >>>> What we need to decide is whether it's okay to drop QEMU "VLANs" >>>> completely and change dump command-line syntax? >>> >>> I'd vote for dropping it. >>> >>>> I think vlan-hub doesn't hurt anyone because the code has been isolated >>>> and we keep backwards compatibility. So I'd personally still go the >>>> vlan-hub route for QEMU 1.x. >>> >>> Just to make it clear: I'm not against this series. I'm against having >>> the functionality in qemu. If we want to keep the functionality, then I >>> completely agree that this series is the way to go. >> >> I agree with Luiz: if we want to reimplement that much of networking >> within QEMU, this series does it in a much better way than VLANs, but >> I'd rather not do it at all. >> >> Just advice, not a strong objection. > > Doesn't the same logic apply to reimplementing file systems? > Shouldn't we drop qcow3 in favor of using btrfs? btrfs isn't ready for production, so this is a hypothetical question. > It's easy to make the NIH argument when it's a feature you don't care about. > > A lot of people use vlans. It's the only way -net socket is useful > too. Just because most KVM/libvirt users don't doesn't mean they > aren't an important feature to preserve. I specifically asked for evidence on actual use of VLANs, and which uses of VLANs can't be readily upgraded to better-performing external solutions. You asserting it is used "a lot" isn't a full answer, but it's (slightly) better than nothing. > I would strongly nack any attempt to remove vlans w/o providing some > mechanism for backwards compatibility which is exactly what this patch > series does. Roma locuta, causa finita. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v3 00/16] net: hub-based networking
Luiz Capitulino writes: > On Mon, 28 May 2012 12:17:04 +0100 > Stefan Hajnoczi wrote: > >> What we need to decide is whether it's okay to drop QEMU "VLANs" >> completely and change dump command-line syntax? > > I'd vote for dropping it. > >> I think vlan-hub doesn't hurt anyone because the code has been isolated >> and we keep backwards compatibility. So I'd personally still go the >> vlan-hub route for QEMU 1.x. > > Just to make it clear: I'm not against this series. I'm against having > the functionality in qemu. If we want to keep the functionality, then I > completely agree that this series is the way to go. I agree with Luiz: if we want to reimplement that much of networking within QEMU, this series does it in a much better way than VLANs, but I'd rather not do it at all. Just advice, not a strong objection. [...] -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v3 00/16] net: hub-based networking
Stefan Hajnoczi writes: > On Fri, May 25, 2012 at 12:18 PM, Markus Armbruster wrote: >> Stefan Hajnoczi writes: >> >>> On Thu, May 24, 2012 at 05:53:21PM -0300, Luiz Capitulino wrote: >>>> On Fri, 25 May 2012 01:59:06 +0800 >>>> zwu.ker...@gmail.com wrote: >>>> >>>> > From: Zhi Yong Wu >>>> > >>>> > The patchset implements network hub stead of vlan. The main work was >>>> > done by stefan, and i rebased it to latest QEMU upstream, did some >>>> > testings and am responsible for pushing it to QEMU upstream. >>>> >>>> Honest question: does it really pay off to have this in qemu vs. using one >>>> of >>>> the externaly available solutions? >>> >>> For typical KVM setups this feature will be unused. >>> >>> However, the legacy QEMU "vlan" feature does have a few uses: >>> >>> 1. It's how the "dump" netdev can be connected up with a guest NIC and >>> host netdev. Create a hub, attach the guest NIC, attach the host >>> netdev, and attach the dump netdev. This allows the dump netdev to >>> see all traffic. >>> >>> 2. It lets you build virtual network segments. Several point-to-point >>> connections can be brought together. Start 3 VMs connected by the >>> "socket" netdev and have one of them use a hub. This may be >>> inefficient but I wouldn't be surprised if there are people out there >>> doing this. >> >> Those people will find other, superior ways to do this once this >> inefficient way is gone. We'd do them a favour, I'd say ;) >> >>> The point of this patch series is to remove the special-case net.c code >>> for the legacy "vlan" feature. Today's code makes it harder to >>> implement a clean QOM model and is a burden for the net subsystem in >>> general. This series takes the vlan functionality and puts it into a >>> normal netdev - it extracts the feature from net.c. >> >> Welcome improvement, but... >> >>> (If we didn't care about backwards compatibility we could just drop >>> vlans completely and rewrite the "dump" netdev to hook into the net.h >>> API somehow.) >> >> ... is backward compatiblity really worth the extra net client? >> >> Please excuse my ignorant question (I haven't studied the series): what >> kind of backward compatiblity exactly do we get here? Command line: >> -net option vlan still works? Or just semantic: whatever you could do >> with vlans you can now do with hubs? >> >> In case it's the latter: well, whatever you could do with vlans you can >> already do with external software, can't you? Why is it worthwhile to >> provide yet another way within QEMU? > > The aim is to keep the command-line vlan= syntax in tact. The hub > change is mostly internal. So it's an extra net client plus somewhat baroque magic to select this client when needed. And when the magic activates, performance suffers. The initiated will expect that, but it'll likely baffle mere users. We can document the issue, but mere users generally don't read documentation. > I agree it would be nice to drop entirely but I don't feel happy doing > that to users who might have QEMU buried in scripts somewhere. One > day they upgrade packages and suddenly their stuff doesn't work > anymore. The keywords is "might". Before we bend over backwards, I'd prefer to see some evidence of anyone actually profiting from it. Anyway, maintainer judgement call. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v3 00/16] net: hub-based networking
Stefan Hajnoczi writes: > On Thu, May 24, 2012 at 05:53:21PM -0300, Luiz Capitulino wrote: >> On Fri, 25 May 2012 01:59:06 +0800 >> zwu.ker...@gmail.com wrote: >> >> > From: Zhi Yong Wu >> > >> > The patchset implements network hub stead of vlan. The main work was done >> > by stefan, and i rebased it to latest QEMU upstream, did some testings and >> > am responsible for pushing it to QEMU upstream. >> >> Honest question: does it really pay off to have this in qemu vs. using one of >> the externaly available solutions? > > For typical KVM setups this feature will be unused. > > However, the legacy QEMU "vlan" feature does have a few uses: > > 1. It's how the "dump" netdev can be connected up with a guest NIC and >host netdev. Create a hub, attach the guest NIC, attach the host >netdev, and attach the dump netdev. This allows the dump netdev to >see all traffic. > > 2. It lets you build virtual network segments. Several point-to-point >connections can be brought together. Start 3 VMs connected by the >"socket" netdev and have one of them use a hub. This may be >inefficient but I wouldn't be surprised if there are people out there >doing this. Those people will find other, superior ways to do this once this inefficient way is gone. We'd do them a favour, I'd say ;) > The point of this patch series is to remove the special-case net.c code > for the legacy "vlan" feature. Today's code makes it harder to > implement a clean QOM model and is a burden for the net subsystem in > general. This series takes the vlan functionality and puts it into a > normal netdev - it extracts the feature from net.c. Welcome improvement, but... > (If we didn't care about backwards compatibility we could just drop > vlans completely and rewrite the "dump" netdev to hook into the net.h > API somehow.) ... is backward compatiblity really worth the extra net client? Please excuse my ignorant question (I haven't studied the series): what kind of backward compatiblity exactly do we get here? Command line: -net option vlan still works? Or just semantic: whatever you could do with vlans you can now do with hubs? In case it's the latter: well, whatever you could do with vlans you can already do with external software, can't you? Why is it worthwhile to provide yet another way within QEMU? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: QEMU-KVM 'Random' Virtual MAC Address.
Veruca Salt writes: > Hello. > When creating an NIC in qemu, I have found that the default address on XP > virtual machines is the same for two VM's on different physical boxes. > Before we go off building an assignment programme to retrofit VM's upgraded > to the fit we're planning, will differently imaged VM's, with different > Computer Names and other data, produce different randomised VMAC's, or will > they always set to the standard default? > > If they do, we are in a happier place already. The default MAC address is 52:54:00:12:34:N, where N is 0x56 for the first NIC, 0x57 for the second, and so forth. Best not to rely on the order of NIC initialization. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: KVM call agenda for April, Tuesday 10
As there are no topics, call is cancelled. Sorry for the late notice. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
KVM call minutes April 3
I'm afraid my notes are rather rough... * 1.1 soft freeze apr 15th (less than two weeks) hard freeze may 1 three months cycle for 1.2 stable machine types only every few releases? "pc-next" * Maintainers, got distracted and my notes make no sense, sorry * MSI injection to KVM irqchips from userspace devices models * qemu-kvm tree: working towards upstream merge not much left, mostly device assignment * Migration: vmstate and visitors, decoupling the wire format why not ASN.1 * qtest: test cases wanted -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] live migration between qemu-kvm 1.0 and 0.15
Anthony Liguori writes: > So, since we're approaching 1.1, we should really discuss release > criteria for 1.1 with respect to live migration. I'd prefer to avoid > surprises in this release. > > My expectation is that migration works from: > > qemu-1.0 -M 1.0 =>qemu-1.1 -M 1.1 > qemu-1.1 -M 1.0 <=qemu-1.1 -M 1.0 > > I would expect that migration works from: > > qemu-0.15 -M 0.15 => qemu-1.1 -M 0.15 > > I'm okay if this fails gracefully: > > qemu-1.1 -M 0.15<= qemu-0.15 -M 0.15 Until we have tools to mechanically verify migratability, such expecations will remain just that: expectations :) Vmstate describes the migration format (sort of), vmstate is data, computers can process data, so at least some mechanical verification should be possible, shouldn't it? And even something that can only disprove migratability, but not prove it, could be incredibly useful. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fixup e432cef9 (aio help text): end sentences with periods
Laszlo Ersek writes: > (Please keep me CC'd on any followup; I'm not subscribed. Thanks.) Patch is fine, but it needs to go to . -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call agenda for Tuesday 24
Anthony Liguori writes: > On 01/23/2012 11:38 AM, Markus Armbruster wrote: >> Please send in any agenda items you are interested in covering. > > I don't have anything pressing. I vote to cancel the call. Call's cancelled. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
KVM call agenda for Tuesday 24
Please send in any agenda items you are interested in covering. Cheers, Markus -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call minutes for November 29
Avi Kivity writes: > On 11/29/2011 05:51 PM, Juan Quintela wrote: >> How to do high level stuff? >> - python? >> > > One of the disadvantages of the various scripting languages is the lack > of static type checking, which makes it harder to do full sweeps of the > source for API changes, relying on the compiler to catch type (or other) > errors. > > On the other hand, the statically typed languages usually have more > boilerplate. Since one of the goals is to simplify things, this > indicates the need for a language with type inference. > > On the third hand, languages with type inferences are still immature > (golang?), so we probably need to keep this discussion going until an > obvious choice presents itself. I wouldn't call ML immature. But I wouldn't call it a scripting language, either. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] kvm-tpr-opt: Fix instruction_is_ok() for push tpr
Missing break spotted by Coverity. Signed-off-by: Markus Armbruster --- kvm-tpr-opt.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/kvm-tpr-opt.c b/kvm-tpr-opt.c index 4b2bd47..14c4b36 100644 --- a/kvm-tpr-opt.c +++ b/kvm-tpr-opt.c @@ -152,6 +152,7 @@ static int instruction_is_ok(CPUState *env, uint64_t rip, int is_write) if (modrm_reg(b2) != 6 || !is_abs_modrm(b2)) return 0; addr_offset = 2; +break; default: return 0; } -- 1.7.6.4 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/GIT PULL] Linux KVM tool for v3.2
Pekka Enberg writes: > Hi Anthony, > > On Thu, Nov 10, 2011 at 3:43 PM, Anthony Liguori > wrote: >> It's not just the qcow2 implementation or even the block layer. This pull >> requests adds a userspace TCP/IP stack to the kernel and yet netdev isn't on >> the CC and there are no Ack's from anyone from the networking stack. I'm >> fairly sure if they knew what was happening here they would object. > > It's something we consider extremely important because it allows easy > non-root networking. But you're right, we definitely ought to ping the > networking folks before the next merge window. The problem is real. The solution "duplicate in user space" sucks. If you engaging with the kernel networking folks leads to one that doesn't suck, we should bathe you in free beer. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/GIT PULL] Linux KVM tool for v3.2
Sasha Levin writes: > On Thu, Nov 10, 2011 at 10:57 AM, Markus Armbruster wrote: >> Sasha Levin writes: [...] >>> I'm actually not sure why KVM tool got QCOW support in the first >>> place. You can have anything QCOW provides if you use btrfs (among >>> several other FSs). >> >> Maybe it's just me, but isn't it weird to have a filesystem (QCOW2) >> sitting in the kernel sources that you can't mount(2)? >> > > It's not really a filesystem, it's a disk image :) Sloppy language on my part, sorry about that. It's a transport for blocks. We have a few of those in the kernel already: block devices. Including loop devices and DRBD. You use a filesystem to interpret their contents. The resulting stack is what gets mounted. Adding another transport for blocks to the kernel that cannot be used that way strikes me as weird. [...] -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/GIT PULL] Linux KVM tool for v3.2
Sasha Levin writes: > On Thu, Nov 10, 2011 at 9:57 AM, Markus Armbruster wrote: [...] >> Start with a clean read/write raw image. Probing declares it raw. >> Guest writes QCOW signature to it, with a backing file of its choice. >> >> Restart with the same image. Probing declares it QCOW2. Guest can read >> the backing file. Oops. > > Thats an excellent scenario why you'd want to have 'Secure KVM' with > seccomp filters :) Yup. For what it's worth, sVirt (use SELinux to secure virtualization) mitigates the problem. Doesn't mean we couldn't use "Secure KVM". > I'm actually not sure why KVM tool got QCOW support in the first > place. You can have anything QCOW provides if you use btrfs (among > several other FSs). Maybe it's just me, but isn't it weird to have a filesystem (QCOW2) sitting in the kernel sources that you can't mount(2)? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/GIT PULL] Linux KVM tool for v3.2
Pekka Enberg writes: > Hi Anthony, > >> On 11/04/2011 03:38 AM, Pekka Enberg wrote: >>> Hi Linus, >>> >>> Please consider pulling the latest KVM tool tree from: >>> >>> git://git.kernel.org/pub/scm/linux/kernel/git/penberg/linux.git >>> kvmtool/for-linus >>> >> >> [snip] >> >>> tools/kvm/virtio/net.c | 423 >>> tools/kvm/virtio/pci.c | 319 ++ >>> tools/kvm/virtio/rng.c | 185 >>> 186 files changed, 19071 insertions(+), 179 deletions(-) > > On Wed, 9 Nov 2011, Anthony Liguori wrote: >> So let's assume for a moment that a tool like this should live in >> the kernel. What's disturbing about a PULL request like this is the >> lack of reviewability of it and the lack of any real review from >> people that understand what's going on in this code base. >> >> There are no Acked-by's by people that really understand what the >> code is doing or that have domain expertise in filesystems and >> networking. >> >> There are major functionality short comings in this code base, data >> corruptors, and CVEs. I'm not saying that the kvm-tool developers >> are bad developers, but the code is not at the appropriate quality >> level for the kernel. It just looks pretty on the surface to people >> that are used to the kernel coding style. >> >> To highlight a few of the issues: [...] >> 2) The qcow2 code is a filesystem implemented in userspace. Image >> formats are file systems. It really should be reviewed by the >> filesystem maintainers. There is absolutely no attempt made to >> synchronize the metadata during write operations which means that >> you do not have crash consistency of the meta data. >> >> If you experience a power failure or kvm-tool crashs, your image >> will get corrupted. I highly doubt a file system would ever be >> merged into Linux that was this naive about data integrity. > > The QCOW2 is lagging behind because we lost the main developer. It's > forced as read-only for the issues you mention. If you think it's a > merge blocker, we can drop it completely from the tree until the > issues are sorted out. > > I personally don't see the issue of having it as a read-only filesystem. > >> 3) The block probing code replicates a well known CVE from three >> years ago[1]. Using kvm-tool, a malicious guest could write the >> qcow2 signature to the zero sector and use that to attack the host. > > We don't support QCOW2 snapshots so I don't see how the "arbitrary > file" thing can happen. You don't need snapshots for the hole. Start with a clean read/write raw image. Probing declares it raw. Guest writes QCOW signature to it, with a backing file of its choice. Restart with the same image. Probing declares it QCOW2. Guest can read the backing file. Oops. Probing images works when all image types can be probed reliably, and the guest can't mess with the probing. Requires distinctive signatures the guest can't change. Raw images spoil it. > It's pretty sad though that we're replicating a known security issue :-/ Maybe I'm wrong, but I got the impression you've been replicating quite a few of QEMU's early mistakes. I hope you can create something better than QEMU, I really, really do. But to successfully build a second system, you need to learn the right lessons from the first system. Are you sure you do? >> [1] http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2008-2004 [...] -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [qemu bug] device assignment doesn't work: "error: requires KVM support"
"Ren, Yongjie" writes: >> -Original Message- >> From: kvm-ow...@vger.kernel.org [mailto:kvm-ow...@vger.kernel.org] On >> Behalf Of Ren, Yongjie >> Sent: Thursday, October 27, 2011 3:03 PM >> To: Markus Armbruster >> Cc: Alex Williamson; KVM General; Avi Kivity >> Subject: RE: [qemu bug] device assignment doesn't work: "error: requires KVM >> support" >> >> > -Original Message- >> > From: Markus Armbruster [mailto:arm...@redhat.com] >> > Sent: Thursday, October 27, 2011 2:50 PM >> > To: Ren, Yongjie >> > Cc: Alex Williamson; KVM General; Avi Kivity >> > Subject: Re: [qemu bug] device assignment doesn't work: "error: requires >> > KVM >> > support" >> > >> > "Ren, Yongjie" writes: >> > >> > >> -Original Message- >> > >> From: Alex Williamson [mailto:alex.william...@redhat.com] >> > >> Sent: Thursday, October 27, 2011 11:04 AM >> > >> To: Ren, Yongjie >> > >> Cc: KVM General; Avi Kivity >> > >> Subject: Re: [qemu bug] device assignment doesn't work: "error: requires >> > KVM >> > >> support" >> > [...] >> > >> The error message is correct, device assignment requires kvm support. >> > >> What does 'info kvm' in the monitor report? >> > > It shows "kvm support: disabled". >> > > But when configuring qemu, I get this message " KVM support yes". See >> details >> > in my attachment configure.log >> > >> > Try running with --enable-kvm. >> >> I added '--enable-kvm' when configuring qemu. I got the same configure >> output. >> And I compiled and ran qemu, then I met the same issue as I describled >> before. > Oh, adding '--enable-kvm' when running qemu will work. The following command > line enables kvm when qemu starting. > ./x86_64-softmmu/qemu-system-x86_64 -smp 2 -m 1024 -device > pci-assign,host=0e:00.0 -hda /root/rhel6u1.img --enable-kvm > But it didn't need to add '--enable-kvm' two weeks ago. Must we add this > parameter from now on if we want to use kvm ? Use of KVM should still be the default. If it isn't anymore, can you use bisect to finger the commit that broke it? [...] -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [qemu bug] device assignment doesn't work: "error: requires KVM support"
"Ren, Yongjie" writes: >> -Original Message- >> From: Markus Armbruster [mailto:arm...@redhat.com] >> Sent: Thursday, October 27, 2011 2:50 PM >> To: Ren, Yongjie >> Cc: Alex Williamson; KVM General; Avi Kivity >> Subject: Re: [qemu bug] device assignment doesn't work: "error: requires KVM >> support" >> >> "Ren, Yongjie" writes: >> >> >> -Original Message- >> >> From: Alex Williamson [mailto:alex.william...@redhat.com] >> >> Sent: Thursday, October 27, 2011 11:04 AM >> >> To: Ren, Yongjie >> >> Cc: KVM General; Avi Kivity >> >> Subject: Re: [qemu bug] device assignment doesn't work: "error: requires >> KVM >> >> support" >> [...] >> >> The error message is correct, device assignment requires kvm support. >> >> What does 'info kvm' in the monitor report? >> > It shows "kvm support: disabled". >> > But when configuring qemu, I get this message " KVM support yes". See >> > details >> in my attachment configure.log >> >> Try running with --enable-kvm. > > I added '--enable-kvm' when configuring qemu. I got the same configure > output. And I compiled and ran qemu, then I met the same issue as I > describled before. Sorry, I meant run qemu --enable-kvm. >> Check /dev/kvm permissions. > I have the permission for /dev/kvm. I use 'root' account. > ls -l /dev/kvm > crw-rw 1 root root 10, 232 Oct 26 22:48 /dev/kvm > > yongjie -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [qemu bug] device assignment doesn't work: "error: requires KVM support"
"Ren, Yongjie" writes: >> -Original Message- >> From: Alex Williamson [mailto:alex.william...@redhat.com] >> Sent: Thursday, October 27, 2011 11:04 AM >> To: Ren, Yongjie >> Cc: KVM General; Avi Kivity >> Subject: Re: [qemu bug] device assignment doesn't work: "error: requires KVM >> support" [...] >> The error message is correct, device assignment requires kvm support. >> What does 'info kvm' in the monitor report? > It shows "kvm support: disabled". > But when configuring qemu, I get this message " KVM support yes". See details > in my attachment configure.log Try running with --enable-kvm. Check /dev/kvm permissions. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call agenda for October 25
Paolo Bonzini writes: > On 10/26/2011 10:48 AM, Markus Armbruster wrote: >> Sector size is a device property. >> >> If the user asks for a 4K sector disk, and the backend can't support >> that, we need to reject the configuration. Just like we reject >> read-only backends for read/write disks. > > Isn't it the other way round, i.e. the user asks for a 512-byte sector > disk (i.e. the default) with cache=none but the disk has 4k sectors? Let me rephrase: If the user asks for a FOO disk, and the backend can't support that, we need to reject the configuration. Just like we reject read-only backends for read/write disks. > We're basically saying "choose between NFS and migration if you have > 4k sector disks but your guest doesn't support them". Understandable > perhaps, but not exactly kind, and virtualization is also about > shielding from this kind of hardware dependency even at the cost of > performance. QEMU should just warn about performance degradations, > erroring out would be a policy decision that should be up to > management. I don't have strong opinions on that. >> It's okay to default device properties to some backend-dependent value, >> if that improves usability. > > On the other hand, not all guests support 4k-sectors properly. You can't pick perfect defaults for all conceivable guests. Life's tough. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call agenda for October 25
Kevin Wolf writes: > Am 25.10.2011 16:06, schrieb Anthony Liguori: >> On 10/25/2011 08:56 AM, Kevin Wolf wrote: >>> Am 25.10.2011 15:05, schrieb Anthony Liguori: I'd be much more open to changing the default mode to cache=none FWIW since the risk of data loss there is much, much lower. >>> >>> I think people said that they'd rather not have cache=none as default >>> because O_DIRECT doesn't work everywhere. >> >> Where doesn't it work these days? I know it doesn't work on tmpfs. I know >> it >> works on ext[234], btrfs, nfs. > > Besides file systems (and probably OSes) that don't support O_DIRECT, > there's another case: Our defaults don't work on 4k sector disks today. > You need to explicitly specify the logical_block_size qdev property for > cache=none to work on them. > > And changing this default isn't trivial as the right value doesn't only > depend on the host disk, but it's also guest visible. The only way out > would be bounce buffers, but I'm not sure that doing that silently is a > good idea... Sector size is a device property. If the user asks for a 4K sector disk, and the backend can't support that, we need to reject the configuration. Just like we reject read-only backends for read/write disks. If the backend can only support it by using bounce buffers, I'd say reject it unless the user explicitly permits bounce buffers. But that's debatable. It's okay to default device properties to some backend-dependent value, if that improves usability. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 6/6] scsi-disk: Check for supported commands
Christoph Hellwig writes: > On Fri, Jul 22, 2011 at 04:51:17PM +0200, Hannes Reinecke wrote: >> Not every command is support for any device type. This patch adds >> a check for rejecting unsupported commands. >> >> Signed-off-by: Hannes Reinecke > > This seems to conflic with Markus' series. But if we want to invest > any major effort into it, we really need to different dispatch tables > for different device types. There's two sane ways to do it: > > one top-level handler with a switch per device type, or tables > with a handler pointer with a device type. I'm fine with either one. > > What I really don't get with this patch is the listing of all the different > SCSI device types. It's already a mistake that we tried to handle disks > and CDROMs with the same driver, but adding even more just makes it worth. It fits into what we have... > IMHO we should simply have one file per SCSI spec, e.g. an spc.c for the > common bits, then an sbc.c for disks, and mmc.c for cdroms. Maybe more > the day we add more emulated device types. Commit b443ae67 `scsi: Split qdev "scsi-disk" into "scsi-hd" and "scsi-cd"' was a first baby step towards that goal. Munging everything together may look like an easy way to avoid code duplication initially, but leads to "common" code riddled with special cases. Proper code reuse among the separate drivers will have to be enforced. Anyway, back to the patch's topic: ideas on how to reject SCSI commands invalid for the device type given the current state of affairs, i.e. disks and CD-ROMs munged together in scsi-disk.c? "Don't, change state of affairs first" is a valid answer, it just means we probably won't get them rejected any time soon. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 5/6] scsi-disk: Remove 'drive_kind'
Hannes Reinecke writes:
> On 07/25/2011 05:59 PM, Markus Armbruster wrote:
>> Hannes Reinecke writes:
>>
>>> Instead of using its own definitions scsi-disk should
>>> be using the device type of the parent device.
>>>
>>> Signed-off-by: Hannes Reinecke
>>> ---
>>> hw/scsi-defs.h |6 +-
>>> hw/scsi-disk.c | 48
>>> 2 files changed, 29 insertions(+), 25 deletions(-)
>>>
> [ .. ]
>>> @@ -1217,44 +1214,47 @@ static int scsi_initfn(SCSIDevice *dev,
>>> SCSIDriveKind kind)
>>> return -1;
>>> }
>>>
>>> -if (kind == SCSI_CD) {
>>> +if (scsi_type == TYPE_ROM) {
>>> s->qdev.blocksize = 2048;
>>> -} else {
>>> +} else if (scsi_type == TYPE_DISK) {
>>> s->qdev.blocksize = s->qdev.conf.logical_block_size;
>>> +} else {
>>> +error_report("scsi-disk: Unhandled SCSI type %02x", scsi_type);
>>> +return -1;
>>> }
>>> s->cluster_size = s->qdev.blocksize / 512;
>>> s->bs->buffer_alignment = s->qdev.blocksize;
>>>
>>> -s->qdev.type = TYPE_DISK;
>>> +s->qdev.type = scsi_type;
>>
>> Is this a bug fix?
>>
> No, proper initialisation.
> s->qdev.type is the SCSI type we're told to emulate. So we have to set
> it to the correct value otherwise the emulation will return wrong
> values.
Well, it changes .type from TYPE_DISK to TYPE_ROM for scsi-cd. I
understand how that is required for your patch to work. I wonder
whether it has an impact beyond that, given that the old value is
plainly wrong.
>>> qemu_add_vm_change_state_handler(scsi_dma_restart_cb, s);
>>> -bdrv_set_removable(s->bs, kind == SCSI_CD);
>>> +bdrv_set_removable(s->bs, scsi_type == TYPE_ROM);
>>> add_boot_device_path(s->qdev.conf.bootindex,&dev->qdev, ",0");
>>> return 0;
>>> }
>>>
>>> static int scsi_hd_initfn(SCSIDevice *dev)
>>> {
>>> -return scsi_initfn(dev, SCSI_HD);
>>> +return scsi_initfn(dev, TYPE_DISK);
>>> }
>>>
>>> static int scsi_cd_initfn(SCSIDevice *dev)
>>> {
>>> -return scsi_initfn(dev, SCSI_CD);
>>> +return scsi_initfn(dev, TYPE_ROM);
>>> }
>>>
>>> static int scsi_disk_initfn(SCSIDevice *dev)
>>> {
>>> -SCSIDriveKind kind;
>>> DriveInfo *dinfo;
>>> +uint8_t scsi_type = TYPE_DISK;
>>>
>>> -if (!dev->conf.bs) {
>>> -kind = SCSI_HD; /* will die in scsi_initfn() */
>>
>> The comment explains why we don't explicitly fail when !dev->conf.bs,
>> like all the other block device models. I'd rather keep it.
>>
> Ah. The magic of block devices. By all means, keep it.
Like this?
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index f42a5d1..0925944 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1251,17 +1251,17 @@ static int scsi_cd_initfn(SCSIDevice *dev)
static int scsi_disk_initfn(SCSIDevice *dev)
{
-SCSIDriveKind kind;
+uint8_t scsi_type;
DriveInfo *dinfo;
if (!dev->conf.bs) {
-kind = SCSI_HD; /* will die in scsi_initfn() */
+scsi_type = TYPE_DISK; /* will die in scsi_initfn() */
} else {
dinfo = drive_get_by_blockdev(dev->conf.bs);
-kind = dinfo->media_cd ? SCSI_CD : SCSI_HD;
+scsi_type = dinfo->media_cd ? TYPE_ROM : TYPE_DISK;
}
-return scsi_initfn(dev, kind);
+return scsi_initfn(dev, scsi_type);
}
#define DEFINE_SCSI_DISK_PROPERTIES() \
By the way, I'm afraid you forgot to remove typedef SCSIDriveKind. Care
to respin this one?
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 0/6][v2] Check for supported SCSI commands
Hannes Reinecke writes: > Markus Armbruster pointed out that not every SCSI command is supported > for a given device type. Based on his patch and suggestiongs this series > cleans up the SCSI device type and adds a check for supported commands. I like this series. It conflicts with mine. I can work with Kevin to resolve the conflict. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 5/6] scsi-disk: Remove 'drive_kind'
Hannes Reinecke writes:
> Instead of using its own definitions scsi-disk should
> be using the device type of the parent device.
>
> Signed-off-by: Hannes Reinecke
> ---
> hw/scsi-defs.h |6 +-
> hw/scsi-disk.c | 48
> 2 files changed, 29 insertions(+), 25 deletions(-)
>
> diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h
> index f644860..27010b7 100644
> --- a/hw/scsi-defs.h
> +++ b/hw/scsi-defs.h
> @@ -164,6 +164,7 @@
>
> #define TYPE_DISK 0x00
> #define TYPE_TAPE 0x01
> +#define TYPE_PRINTER0x02
> #define TYPE_PROCESSOR 0x03/* HP scanners use this */
> #define TYPE_WORM 0x04/* Treated as ROM by our system */
> #define TYPE_ROM0x05
> @@ -171,6 +172,9 @@
> #define TYPE_MOD0x07/* Magneto-optical disk -
>* - treated as TYPE_DISK */
> #define TYPE_MEDIUM_CHANGER 0x08
> -#define TYPE_ENCLOSURE 0x0d/* Enclosure Services Device */
> +#define TYPE_STORAGE_ARRAY 0x0c/* Storage array device */
> +#define TYPE_ENCLOSURE 0x0d/* Enclosure Services Device */
> +#define TYPE_RBC0x0e/* Simplified Direct-Access Device */
> +#define TYPE_OSD0x11/* Object-storage Device */
> #define TYPE_NO_LUN 0x7f
>
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index 535fa11..ae2c157 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -74,7 +74,6 @@ struct SCSIDiskState
> char *version;
> char *serial;
> SCSISense sense;
> -SCSIDriveKind drive_kind;
> };
>
> static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type);
> @@ -382,7 +381,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
> uint8_t *outbuf)
> return -1;
> }
>
> -if (s->drive_kind == SCSI_CD) {
> +if (s->qdev.type == TYPE_ROM) {
> outbuf[buflen++] = 5;
> } else {
> outbuf[buflen++] = 0;
> @@ -401,7 +400,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
> uint8_t *outbuf)
> if (s->serial)
> outbuf[buflen++] = 0x80; // unit serial number
> outbuf[buflen++] = 0x83; // device identification
> -if (s->drive_kind == SCSI_HD) {
> +if (s->qdev.type == TYPE_DISK) {
> outbuf[buflen++] = 0xb0; // block limits
> outbuf[buflen++] = 0xb2; // thin provisioning
> }
> @@ -460,7 +459,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
> uint8_t *outbuf)
> unsigned int opt_io_size =
> s->qdev.conf.opt_io_size / s->qdev.blocksize;
>
> -if (s->drive_kind == SCSI_CD) {
> +if (s->qdev.type == TYPE_ROM) {
> DPRINTF("Inquiry (EVPD[%02X] not supported for CDROM\n",
> page_code);
> return -1;
> @@ -530,12 +529,11 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
> uint8_t *outbuf)
> return buflen;
> }
>
> -if (s->drive_kind == SCSI_CD) {
> -outbuf[0] = 5;
> +outbuf[0] = s->qdev.type & 0x1f;
> +if (s->qdev.type == TYPE_ROM) {
> outbuf[1] = 0x80;
> memcpy(&outbuf[16], "QEMU CD-ROM ", 16);
> } else {
> -outbuf[0] = 0;
> outbuf[1] = s->removable ? 0x80 : 0;
> memcpy(&outbuf[16], "QEMU HARDDISK ", 16);
> }
> @@ -661,7 +659,7 @@ static int mode_sense_page(SCSIRequest *req, int page,
> uint8_t *p,
> return p[1] + 2;
>
> case 0x2a: /* CD Capabilities and Mechanical Status page. */
> -if (s->drive_kind != SCSI_CD)
> +if (s->qdev.type != TYPE_ROM)
> return 0;
> p[0] = 0x2a;
> p[1] = 0x14;
> @@ -877,7 +875,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r,
> uint8_t *outbuf)
> goto illegal_request;
> break;
> case START_STOP:
> -if (s->drive_kind == SCSI_CD && (req->cmd.buf[4] & 2)) {
> +if (s->qdev.type == TYPE_ROM && (req->cmd.buf[4] & 2)) {
> /* load/eject medium */
> bdrv_eject(s->bs, !(req->cmd.buf[4] & 1));
> }
> @@ -1183,7 +1181,7 @@ static void scsi_destroy(SCSIDevice *dev)
> blockdev_mark_auto_del(s->qdev.conf.bs);
> }
>
> -static int scsi_initfn(SCSIDevice *dev, SCSIDriveKind kind)
> +static int scsi_initfn(SCSIDevice *dev, uint8_t scsi_type)
> {
> SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
> DriveInfo *dinfo;
> @@ -1193,9 +1191,8 @@ static int scsi_initfn(SCSIDevice *dev, SCSIDriveKind
> kind)
> return -1;
> }
> s->bs = s->qdev.conf.bs;
> -s->drive_kind = kind;
>
> -if (kind == SCSI_HD && !bdrv_is_inserted(s->bs)) {
> +if (scsi_type == TYPE_DISK && !bdrv_is_inserted(s->bs)) {
> error_report("Device needs media, but drive is empty");
> return -1;
>
Re: [Qemu-devel] [PATCH] Introduce QEMU_NEW()
Kevin Wolf writes: > Am 25.07.2011 12:06, schrieb Stefan Hajnoczi: >> On Mon, Jul 25, 2011 at 9:51 AM, Avi Kivity wrote: >>> qemu_malloc() is type-unsafe as it returns a void pointer. Introduce >>> QEMU_NEW() (and QEMU_NEWZ()), which return the correct type. >>> >>> Signed-off-by: Avi Kivity >>> --- >>> >>> This is part of my memory API patchset, but doesn't really belong there. >>> >>> qemu-common.h |3 +++ >>> 1 files changed, 3 insertions(+), 0 deletions(-) >>> >>> diff --git a/qemu-common.h b/qemu-common.h >>> index ba55719..66effa3 100644 >>> --- a/qemu-common.h >>> +++ b/qemu-common.h >>> @@ -186,6 +186,9 @@ void qemu_free(void *ptr); >>> char *qemu_strdup(const char *str); >>> char *qemu_strndup(const char *str, size_t size); >>> >>> +#define QEMU_NEW(type) ((type *)(qemu_malloc(sizeof(type >>> +#define QEMU_NEWZ(type) ((type *)(qemu_mallocz(sizeof(type >> >> Does this mean we need to duplicate the type name for each allocation? >> >> struct foo *f; >> >> ... >> f = qemu_malloc(sizeof(*f)); >> >> Becomes: >> >> struct foo *f; >> >> ... >> f = QEMU_NEW(struct foo); > > Maybe we should allow this and make it the usual pattern: > > f = qemu_new(typeof(*f)); > > It's gcc specific, but we already don't care about portability to other > compilers in more places. > > On the other hand, how many bugs did we have recently that were caused > by a wrong sizeof for qemu_malloc? As far as I can say, there's no real > reason to do it. I think it's the same kind of discussion as with > forbidding qemu_malloc(0) (except that this time it just won't improve > things much instead of being really stupid). Side-stepping the stupid "OMG malloc(0) is weird, therefore we must make qemu_malloc(0) differently weird" controversy would be useful all by itself. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH] Introduce QEMU_NEW()
Avi Kivity writes: > On 07/25/2011 01:04 PM, Alexander Graf wrote: >> On 25.07.2011, at 12:02, Avi Kivity wrote: >> >> > On 07/25/2011 12:56 PM, Alexander Graf wrote: >> >> > >> >> > That argument can be used to block any change. You'll get used to >> >> it in time. The question is, is the new interface better or not. >> >> >> >> I agree that it keeps you from accidently malloc'ing a struct of pointer >> >> size. But couldn't we also just add this to checkpatch.pl? >> > >> > Better APIs trump better patch review. >> >> Only if you enforce them. The only sensible thing for QEMU_NEW (despite the >> general rule of upper case macros, I'd actually prefer this one to be lower >> case though since it's so often used) would be to remove qemu_malloc, >> declare malloc() as unusable and convert all users of qemu_malloc() to >> qemu_new(). > > Some qemu_mallocs() will remain (allocating a byte array or something > variable sized). Byte array: add the obvious type-safe allocator for a variable-sized array T[N], then use it with unsigned char for T. In fact, I find QEMU_NEW() pretty pointless without a buddy for arrays. Still not covered: allocating a struct with a variable-size array as final member. I guess a solution for that can be found if we care enough. [...] -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 3/3] scsi-disk: Check for supported commands
Hannes Reinecke writes: > Not every command is support for any device type. This patch adds > a check for rejecting unsupported commands. > > Signed-off-by: Hannes Reinecke Commit message says "patch adds", patch only deletes. Looks like something wrent wrong with 2/3 and 3/3. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 1/3] scsi: Sanitize command definitions
Hannes Reinecke writes:
> Adding some missing command definitions and update the existing ones.
> No functional change.
Add: LOCATE_10, UNMAP, VARLENGTH_CDB, WRITE_FILEMARKS_16, EXTENDED_COPY,
ATA_PASSTHROUGH, ACCESS_CONTROL_IN, ACCESS_CONTROL_OUT,
COMPARE_AND_WRITE, VERIFY_16, SYNCHRONIZE_CACHE_16, LOCATE_16, ERASE_16,
WRITE_LONG_16, VERIFY_12, READ_DEFECT_DATA_12.
Rename: READ_CAPACITY, WRITE_VERIFY, VERIFY, READ_LONG, WRITE_LONG,
WRITE_SAME to &_10.
Delete: WRITE_LONG_2
>
> Signed-off-by: Hannes Reinecke
> ---
> hw/scsi-bus.c | 71 ++--
> hw/scsi-defs.h| 60
> hw/scsi-disk.c| 29 -
> hw/scsi-generic.c |2 +-
> 4 files changed, 91 insertions(+), 71 deletions(-)
>
> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
> index 8b1a412..24a1298 100644
> --- a/hw/scsi-bus.c
> +++ b/hw/scsi-bus.c
> @@ -232,24 +232,24 @@ static int scsi_req_length(SCSIRequest *req, uint8_t
> *cmd)
> case RELEASE:
> case ERASE:
> case ALLOW_MEDIUM_REMOVAL:
> -case VERIFY:
> +case VERIFY_10:
> case SEEK_10:
> case SYNCHRONIZE_CACHE:
> case LOCK_UNLOCK_CACHE:
> case LOAD_UNLOAD:
> case SET_CD_SPEED:
> case SET_LIMITS:
> -case WRITE_LONG:
> +case WRITE_LONG_10:
> case MOVE_MEDIUM:
> case UPDATE_BLOCK:
> req->cmd.xfer = 0;
> break;
> case MODE_SENSE:
> break;
> -case WRITE_SAME:
> +case WRITE_SAME_10:
> req->cmd.xfer = 1;
> break;
> -case READ_CAPACITY:
> +case READ_CAPACITY_10:
> req->cmd.xfer = 8;
> break;
> case READ_BLOCK_LIMITS:
> @@ -265,7 +265,7 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd)
> req->cmd.xfer *= 8;
> break;
> case WRITE_10:
> -case WRITE_VERIFY:
> +case WRITE_VERIFY_10:
> case WRITE_6:
> case WRITE_12:
> case WRITE_VERIFY_12:
> @@ -325,7 +325,7 @@ static void scsi_req_xfer_mode(SCSIRequest *req)
> switch (req->cmd.buf[0]) {
> case WRITE_6:
> case WRITE_10:
> -case WRITE_VERIFY:
> +case WRITE_VERIFY_10:
> case WRITE_12:
> case WRITE_VERIFY_12:
> case WRITE_16:
> @@ -345,15 +345,13 @@ static void scsi_req_xfer_mode(SCSIRequest *req)
> case SEARCH_HIGH:
> case SEARCH_LOW:
> case UPDATE_BLOCK:
> -case WRITE_LONG:
> -case WRITE_SAME:
> +case WRITE_LONG_10:
> +case WRITE_SAME_10:
> case SEARCH_HIGH_12:
> case SEARCH_EQUAL_12:
> case SEARCH_LOW_12:
> -case SET_WINDOW:
I figure this is "no functional change" because all users of this
function reject SET_WINDOW elsewhere.
> case MEDIUM_SCAN:
> case SEND_VOLUME_TAG:
> -case WRITE_LONG_2:
Same here.
> case PERSISTENT_RESERVE_OUT:
> case MAINTENANCE_OUT:
> req->cmd.mode = SCSI_XFER_TO_DEV;
> @@ -517,8 +515,7 @@ static const char *scsi_command_name(uint8_t cmd)
> {
> static const char *names[] = {
> [ TEST_UNIT_READY ] = "TEST_UNIT_READY",
> -[ REZERO_UNIT ] = "REZERO_UNIT",
> -/* REWIND and REZERO_UNIT use the same operation code */
> +[ REWIND ] = "REWIND",
"No functional change" because it's used only for scsi_req_print(),
which is unused.
> [ REQUEST_SENSE] = "REQUEST_SENSE",
> [ FORMAT_UNIT ] = "FORMAT_UNIT",
> [ READ_BLOCK_LIMITS] = "READ_BLOCK_LIMITS",
> @@ -543,14 +540,13 @@ static const char *scsi_command_name(uint8_t cmd)
> [ RECEIVE_DIAGNOSTIC ] = "RECEIVE_DIAGNOSTIC",
> [ SEND_DIAGNOSTIC ] = "SEND_DIAGNOSTIC",
> [ ALLOW_MEDIUM_REMOVAL ] = "ALLOW_MEDIUM_REMOVAL",
> -
> [ SET_WINDOW ] = "SET_WINDOW",
> -[ READ_CAPACITY] = "READ_CAPACITY",
> +[ READ_CAPACITY_10 ] = "READ_CAPACITY_10",
> [ READ_10 ] = "READ_10",
> [ WRITE_10 ] = "WRITE_10",
> [ SEEK_10 ] = "SEEK_10",
New LOCATE_10 missing.
> -[ WRITE_VERIFY ] = "WRITE_VERIFY",
> -[ VERIFY ] = "VERIFY",
> +[ WRITE_VERIFY_10 ] = "WRITE_VERIFY_10",
> +[ VERIFY_10] = "VERIFY_10",
> [ SEARCH_HIGH ] = "SEARCH_HIGH",
> [ SEARCH_EQUAL ] = "SEARCH_EQUAL",
> [ SEARCH_LOW ] = "SEARCH_LOW",
> @@ -566,11 +562,14 @@ static const char *scsi_command_name(uint8_t cmd)
> [ WRITE_BUFFER ] = "WRITE_BUFFER",
> [ READ_BUFFER ] = "READ_BUFFER",
> [ UPDATE_BLOCK ] = "UPDATE_BLOCK",
> -[ READ_LONG] = "READ_LONG",
> -[ WRITE_LONG ] = "WRITE_LONG",
> +[ READ_LONG_10 ] = "REA
Re: [Qemu-devel] [RFC] New thread for the VM migration
Juan Quintela writes: [...] > I have think a little bit about hotplug & migration, and haven't arraive > to a nice solution. > > - Disabling hotplug/unplug during migration: easy to do. But it is not > exactly user friendly (we are here). > > - Allowing hotplug during migration. Solutions: > * allow the transfer of hotplug events over the migration protocol > (make it even more complicated, but not too much. The big problem is > if the hotplug succeeds on the source but not the destination, ...) > * migrate the device list in stage 3: it fixes the hotplug problem > nicely, but it makes the interesting problem that after migrating > all the ram, we can find "interesting" problems like: disk not > readable, etc. Not funny. > * > > As far as I can see, if we sent the device list 1st, we can create the > full machine at destination, but hotplug is "interesting" to manage. > Sending the device list late, solve hotplug, but allows errors after > migrating all memory (also known as, why don't you told me *sooner*). I figure the errors relevant here happen in device backends (host parts) mostly. Maybe updating just backends is easier than full device hot plug. Configure backends before migrating memory, to catch errors. Reconfigure backends afterwards for hot plug[*]. Then build machine. You still get errors from frontends (device models) after migrating memory, but they should be rare. [...] [*] You could do it "in the middle" to catch errors as early as possible, but I doubt it's worth the trouble. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] qemu: Add strtosz_suffix_unit function
Joerg Roedel writes:
> This function does the same as the strtosz_suffix function
> except that it allows to specify the unit to which the
> k/M/B/T suffixes apply. This function will be used later to
> parse the tsc-frequency from the command-line.
>
> Signed-off-by: Joerg Roedel
> ---
> cutils.c | 16 +++-
> qemu-common.h |2 ++
> 2 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/cutils.c b/cutils.c
> index f9a7e36..28049e0 100644
> --- a/cutils.c
> +++ b/cutils.c
> @@ -322,7 +322,8 @@ int fcntl_setfl(int fd, int flag)
> * value must be terminated by whitespace, ',' or '\0'. Return -1 on
> * error.
> */
> -int64_t strtosz_suffix(const char *nptr, char **end, const char
> default_suffix)
> +int64_t strtosz_suffix_unit(const char *nptr, char **end,
> +const char default_suffix, int64_t unit)
> {
> int64_t retval = -1;
> char *endptr;
> @@ -362,20 +363,20 @@ int64_t strtosz_suffix(const char *nptr, char **end,
> const char default_suffix)
> }
> break;
> case STRTOSZ_DEFSUFFIX_KB:
> -mul = 1 << 10;
> +mul = unit;
> break;
> case 0:
> if (mul_required) {
> goto fail;
> }
> case STRTOSZ_DEFSUFFIX_MB:
> -mul = 1ULL << 20;
> +mul = unit * unit;
> break;
> case STRTOSZ_DEFSUFFIX_GB:
> -mul = 1ULL << 30;
> +mul = unit * unit * unit;
> break;
> case STRTOSZ_DEFSUFFIX_TB:
> -mul = 1ULL << 40;
> +mul = unit * unit * unit * unit;
> break;
> default:
> goto fail;
> @@ -405,6 +406,11 @@ fail:
> return retval;
> }
Why would anyone ever call this function with an unit argument other
than 1000 or 1024?
Without such a use case, I'd rather give strtosz_suffix() a flag
parameter to pick SI prefixes (multiples of 1000) vs. binary prefixes
(multiples of 1024).
[...]
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 3/3] Avoid Wunsed-but-set warnings (or errors in case of Werror)
Peter Maydell writes: > On 5 July 2011 07:15, Markus Armbruster wrote: >>> + int fd, __attribute__((unused)) ret; >>> >>> snprintf(reset_file, sizeof(reset_file), >>> "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/reset", >> >> What about (void)write() and do away with ret? > > If 'ret' has been used to silence compiler warnings about functions > which have been declared with attribute __warn_unused_result__ > (eg write() and various other libc functions) then "(void)write()" > is insufficient -- gcc requires the variable. gcc being silly. Oh well. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 3/3] Avoid Wunsed-but-set warnings (or errors in case of Werror)
Typo in subject: "unsed". The warning is spelled
"unused-but-set-variable", the option "-Wunused-but-set-variable".
Raghavendra D Prabhu writes:
> In a few cases, variable attributed 'unused' has been added, in other cases
> unused variable has been either removed or commented out.
>
> Signed-off-by: Raghavendra D Prabhu
> ---
> hw/device-assignment.c |6 +++---
> simpletrace.c |2 +-
> xen-mapcache.c |7 ++-
> 3 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> index 36ad6b0..19a59b4 100644
> --- a/hw/device-assignment.c
> +++ b/hw/device-assignment.c
> @@ -1654,7 +1654,7 @@ static void reset_assigned_device(DeviceState *dev)
> AssignedDevice *adev = DO_UPCAST(AssignedDevice, dev, pci_dev);
> char reset_file[64];
> const char reset[] = "1";
> -int fd, ret;
> +int fd, __attribute__((unused)) ret;
>
> snprintf(reset_file, sizeof(reset_file),
> "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/reset",
What about (void)write() and do away with ret?
> @@ -1682,7 +1682,7 @@ static void reset_assigned_device(DeviceState *dev)
> static int assigned_initfn(struct PCIDevice *pci_dev)
> {
> AssignedDevice *dev = DO_UPCAST(AssignedDevice, dev, pci_dev);
> -uint8_t e_device, e_intx;
> +uint8_t e_intx;
> int r;
>
> if (!kvm_enabled()) {
> @@ -1709,7 +1709,7 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
> goto out;
>
> /* handle interrupt routing */
> -e_device = (dev->dev.devfn >> 3) & 0x1f;
> +/*e_device = (dev->dev.devfn >> 3) & 0x1f;*/
> e_intx = dev->dev.config[0x3d] - 1;
> dev->intpin = e_intx;
> dev->run = 0;
> diff --git a/simpletrace.c b/simpletrace.c
> index f1dbb5e..2ce9cff 100644
> --- a/simpletrace.c
> +++ b/simpletrace.c
> @@ -119,7 +119,7 @@ static void *writeout_thread(void *opaque)
> TraceRecord record;
> unsigned int writeout_idx = 0;
> unsigned int num_available, idx;
> -size_t unused;
> +size_t __attribute__((unused)) unused;
>
> for (;;) {
> wait_for_trace_records_available();
Same here.
[...]
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call agenda for June 21
Avi Kivity writes: > On 06/21/2011 04:50 PM, Anthony Liguori wrote: >> On 06/21/2011 06:28 AM, Avi Kivity wrote: >>> On 06/20/2011 10:42 AM, Juan Quintela wrote: Please send in any agenda items you are interested in covering. >>> >>> >>> https://bugzilla.redhat.com/show_bug.cgi?id=689672 - Guests do not start >>> after upgrading qemu to 0.13 >>> >>> Seems like our backward compatibility plan isn't working. How do we >>> address it? How do we test it? >> >> f13 is ancient, no? > > Yes, a year old. > > Furthermore, Justin tells me it carries a lot downstream patches. > >> >> I'm not sure what this particular issue is, but is this doing -M pc-0.12? >> > > It has its own machine type. So this report may not indicate any > problem with upstream. > > Still, I feel we have a potential problem here. We identify > guest-visible attributes just by review; we're sure to miss something > here and there. Unlike ordinary bugs, compatibility problems only > show up later and are much harder to fix. > > Second, we don't do any tests in this area that I'm aware of. Lucas, > what would it take (thanks, you're most kind) to test multiple qemus > in a single run? > > We can have a script that runs lspci -vvv, x86info, > and other interesting stuff and compare the results, and also system > tests that boot a guest on multiple qemus (with the same -M and > different -M) and see if things work. Suggest to compare info qtree as well. > We can probably continue on email, I don't see a real need for a call > for this topic. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 00/11] cpu model bug fixes and definition corrections (v2)
Jan Kiszka writes: [...] >> Something else I may have missed? >> > > Nothing critical, I'm just hoping someone finds the time to fix > sysconfigs loading when starting qemu from a build directory. :) There's nice to have, important, critical, and really annoying. This one's really annoying. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: differencing disks support
Iordan Iordanov writes: > Does KVM support or plan to support differencing disks (where there is > a read-only source disk, and each person running a virtual machine can > save block-level changes that their virtual machine is making to the > disk in a separate "differencing" image)? > > If so, can somebody suggest how I may make use of this feature > (i.e. building the newest version from source, and any other > requirements). Check out image format qcow2. LVM snapshots were already mentioned. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] testdev: adjust for ISA irq changes
Jan Kiszka writes: > On 2011-05-29 17:26, Avi Kivity wrote: >> On 05/29/2011 06:21 PM, Jan Kiszka wrote: [...] >>> We also need a better interface to discover and track legacy IRQ routes >>> for device assignment. Markus is currently collecting requirements for >>> qdev enhancements, and I think generic IRQ manipulation and discovery >>> belongs there. >> >> Possibly. But note that attempting to shoehorn everything into >> bus/device model may not work. Motherboard devices, especially, often >> bypass the bus/device relationship, just because everything is available >> to them on the motherboard, and because hardware designers didn't go to >> software engineering schools but instead do what's necessary to get >> things working. We have to be prepared for exceptions. > > Yes, the IRQ tree should probably be independent of the qdev tree. I was > thinking of qdev as the infrastructure that allows to build and maintain > that tree and to associate IRQ pins with qdev devices. The bus/device model limits the connections between devices to a tree, which can't model real world interrupt wirings. We obviously need to generalize there. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v2] Add an isa device for SGA
Glauber Costa writes:
> This patch adds a dummy legacy ISA device whose responsibility is to
> deploy sgabios, an option rom for a serial graphics adapter.
> The proposal is that this device is always-on when -nographics,
> but can otherwise be enable in any setup when -device sga is used.
>
> [v2: suggestions on qdev by Markus ]
One more question: should "-device sga" suppress the default VGA? The
other three VGA devices do; check out default_list[] in vl.c.
[...]
> diff --git a/hw/pc.h b/hw/pc.h
> index 1291e2d..a00e054 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -219,6 +219,9 @@ int isa_vga_mm_init(target_phys_addr_t vram_base,
> void pci_cirrus_vga_init(PCIBus *bus);
> void isa_cirrus_vga_init(void);
>
> +/* serial graphics */
> +void isa_sga_init(void);
> +
No longer exists, please drop.
> /* ne2000.c */
> static inline bool isa_ne2000_init(int base, int irq, NICInfo *nd)
> {
[...]
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH] Avoid segmentation fault for qdev device not found
Glauber Costa writes: > qdev_try_create will cope well with a NULL bus, since it will assume > the main system bus by default. qdev_create, however, wants to print > a message, in which it instantiates the bus name. That simple and at > first inoffensive message will generate a segmentation found if the > reason for failure is a NULL bus. > > I propose we avoid that - thus generating the normal hw_error by > always passing a valid bus to qdev_try_create - if none, be it the > main system bus. > > The code for testing a NULL bus is not remove from qdev_try_create > because it is a externally visible function, and we want it to continue > working fine. > > Signed-off-by: Glauber Costa "NULL means implicit main system bus" is too clever. See also commit ec990eb6. Beyond the scope of your simple fix, so: Reviewed-by: Markus Armbruster -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH] Add an isa device for SGA
Glauber Costa writes:
> This patch adds a dummy legacy ISA device whose responsibility is to
> deploy sgabios, an option rom for a serial graphics adapter.
> The proposal is that this device is always-on when -nographics,
> but can otherwise be enable in any setup when -device sga is used.
>
> Signed-off-by: Glauber Costa
> ---
> Makefile.target |2 +-
> hw/pc.c |2 ++
> hw/pc.h |3 +++
> hw/sga.c| 49 +
> 4 files changed, 55 insertions(+), 1 deletions(-)
> create mode 100644 hw/sga.c
>
> diff --git a/Makefile.target b/Makefile.target
> index fdbdc6c..004ea7e 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -224,7 +224,7 @@ obj-$(CONFIG_KVM) += ivshmem.o
> # Hardware support
> obj-i386-y += vga.o
> obj-i386-y += mc146818rtc.o i8259.o pc.o
> -obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o
> +obj-i386-y += cirrus_vga.o sga.o apic.o ioapic.o piix_pci.o
> obj-i386-y += vmport.o
> obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
> obj-i386-y += extboot.o
> diff --git a/hw/pc.c b/hw/pc.c
> index 8d351ba..56c3887 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -1096,6 +1096,8 @@ void pc_vga_init(PCIBus *pci_bus)
> isa_vga_init();
> }
> }
> +
> +isa_sga_init();
Please do this the qdev way (untested):
if (display_type == DT_NOGRAPHIC) {
dev = qdev_create(NULL, "sga");
qdev_init_nofail(dev);
}
> }
>
> static void cpu_request_exit(void *opaque, int irq, int level)
> diff --git a/hw/pc.h b/hw/pc.h
> index 1291e2d..a00e054 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -219,6 +219,9 @@ int isa_vga_mm_init(target_phys_addr_t vram_base,
> void pci_cirrus_vga_init(PCIBus *bus);
> void isa_cirrus_vga_init(void);
>
> +/* serial graphics */
> +void isa_sga_init(void);
> +
> /* ne2000.c */
> static inline bool isa_ne2000_init(int base, int irq, NICInfo *nd)
> {
> diff --git a/hw/sga.c b/hw/sga.c
> new file mode 100644
> index 000..411191b
> --- /dev/null
> +++ b/hw/sga.c
> @@ -0,0 +1,49 @@
> +#include "pci.h"
> +#include "pc.h"
> +#include "loader.h"
> +#include "sysemu.h"
> +
> +#define SGABIOS_FILENAME "sgabios.bin"
> +
> +typedef struct ISAGAState {
> +ISADevice dev;
> +} ISASGAState;
> +
> +/* We can have both -device, and the initfn called, so better
> + * avoid to have the rom loaded twice */
> +static void deploy_rom(void)
> +{
> +static int rom_deployed = 0;
> +
> +if (!rom_deployed++) {
> +rom_add_vga(SGABIOS_FILENAME);
> +}
> +}
Is rom_deployed still needed with isa_sga_init() gone?
> +
> +static int isa_cirrus_vga_initfn(ISADevice *dev)
> +{
> +deploy_rom();
> +
> +return 0;
> +}
> +
> +void isa_sga_init(void)
> +{
> +if (display_type == DT_NOGRAPHIC) {
> +deploy_rom();
> +}
> +}
> +
> +static ISADeviceInfo sga_info = {
> +.qdev.name= "sga",
> +.qdev.desc= "Serial Graphics Adapter",
> +.qdev.size= sizeof(ISASGAState),
> +.init = isa_cirrus_vga_initfn,
> +};
> +
> +static void sga_register(void)
> +{
> + isa_qdev_register(&sga_info);
> +}
> +
> +device_init(sga_register);
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Why QCOW1? (was: [PATCH v2] kvm tool: add QCOW verions 1 read/write support)
Kevin Wolf writes: > Am 13.04.2011 21:26, schrieb Prasad Joshi: >> The patch only implements the basic read write support for QCOW version 1 >> images. Many of the QCOW features are not implmented, for example >> - image creation >> - snapshot >> - copy-on-write >> - encryption > > Yay, more forks, more code duplication! > > Have you thought about a way to actually share code with qemu instead of > repeating Xen's mistake of copying code, modifying it until merges are > no longer possible and then let it bitrot? > > If you shared code, you also wouldn't have to use an obsolete, but > simple-to-implement format, but could use some of the better maintained > formats of qemu, like qcow2. > > Also at least your qcow1.c is lacking the copyright header. Please add > this, otherwise you're violating the license. As discussed already, reimplementing rather than sharing can be excused, because the existing code is not ready for sharing. What hasn't been discussed much is the other half of Kevin's remark: why QCOW1? Does anyone still use that old hag in anger? The format people actually use is QCOW2, and it is properly maintained. Even for little-used QOW formats, there are more interesting choices: QED and FVD. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] kvm tool: add QCOW verions 1 read/write support
Kevin Wolf writes: > Am 14.04.2011 11:26, schrieb Markus Armbruster: >> Kevin Wolf writes: >> >>> Am 14.04.2011 10:32, schrieb Pekka Enberg: >>>> Hi Kevin! >>>> >>>> Am 14.04.2011 10:21, schrieb Pekka Enberg: >>>>>> On Thu, Apr 14, 2011 at 11:02 AM, Kevin Wolf wrote: >>>>>>> Have you thought about a way to actually share code with qemu instead of >>>>>>> repeating Xen's mistake of copying code, modifying it until merges are >>>>>>> no longer possible and then let it bitrot? >>>>>> >>>>>> No we haven't and we're not planning to copy QEMU code as-is but >>>>>> re-implement support for formats we're interested in. >>>> >>>> On Thu, Apr 14, 2011 at 11:31 AM, Kevin Wolf wrote: >>>>> Okay. I might not consider it wise, but in the end it's your decision. >>>>> I'm just curious why you think this is the better way? >>>> >>>> Well, how would you go about sharing the code without copying in >>>> practical terms? We're aiming for standalone tool in tools/kvm of the >>>> Linux kernel so I don't see how we could do that. >>> >>> Well, copying in itself is not a big problem as long as the copies are >>> kept in sync. It's a bit painful, but manageable. Implementing every >>> image format twice (and implementing image formats in a reliable and >>> performing way isn't trivial) is much more painful. >>> >>> If you take the approach of "getting inspired" by qemu and then writing >>> your own code, the code will read pretty much the same, but be different >>> enough that a diff between both trees is useless and a patch against one >>> tree is meaningless for the other one. >>> >>> The block drivers are relatively isolated in qemu, so I think they >>> wouldn't pull in too many dependencies. >> >> Are you suggesting to turn QEMU's block drivers into a reasonably >> general-purpose library? > > I would hesitate to turn it into an external library, because I really > don't feel like maintaining API compatibility across versions. That's > simply not doable with the block layer as of today. For the long term > it's something that we may consider, but would certainly require some > serious work. > > If some changes are needed to make it more reusable in the short term > (while still copying the code over), I probably wouldn't be opposed to that. Unless we make QEMU's block drivers usable outside QEMU (and that means at least a static library without API guarantees), we can hardly chide others for reimplementing them. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] kvm tool: add QCOW verions 1 read/write support
Kevin Wolf writes: > Am 14.04.2011 10:32, schrieb Pekka Enberg: >> Hi Kevin! >> >> Am 14.04.2011 10:21, schrieb Pekka Enberg: On Thu, Apr 14, 2011 at 11:02 AM, Kevin Wolf wrote: > Have you thought about a way to actually share code with qemu instead of > repeating Xen's mistake of copying code, modifying it until merges are > no longer possible and then let it bitrot? No we haven't and we're not planning to copy QEMU code as-is but re-implement support for formats we're interested in. >> >> On Thu, Apr 14, 2011 at 11:31 AM, Kevin Wolf wrote: >>> Okay. I might not consider it wise, but in the end it's your decision. >>> I'm just curious why you think this is the better way? >> >> Well, how would you go about sharing the code without copying in >> practical terms? We're aiming for standalone tool in tools/kvm of the >> Linux kernel so I don't see how we could do that. > > Well, copying in itself is not a big problem as long as the copies are > kept in sync. It's a bit painful, but manageable. Implementing every > image format twice (and implementing image formats in a reliable and > performing way isn't trivial) is much more painful. > > If you take the approach of "getting inspired" by qemu and then writing > your own code, the code will read pretty much the same, but be different > enough that a diff between both trees is useless and a patch against one > tree is meaningless for the other one. > > The block drivers are relatively isolated in qemu, so I think they > wouldn't pull in too many dependencies. Are you suggesting to turn QEMU's block drivers into a reasonably general-purpose library? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 2/2 V7] qemu, qmp: add inject-nmi qmp command
Blue Swirl writes: > On Wed, Apr 13, 2011 at 4:08 PM, Luiz Capitulino > wrote: >> On Tue, 12 Apr 2011 21:31:18 +0300 >> Blue Swirl wrote: >> >>> On Tue, Apr 12, 2011 at 10:52 AM, Avi Kivity wrote: >>> > On 04/11/2011 08:15 PM, Blue Swirl wrote: >>> >> >>> >> On Mon, Apr 11, 2011 at 10:01 AM, Markus Armbruster >>> >> wrote: >>> >> > Avi Kivity writes: >>> >> > >>> >> >> On 04/08/2011 12:41 AM, Anthony Liguori wrote: >>> >> >>> >>> >> >>> And it's a good thing to have, but exposing this as the only API to >>> >> >>> do something as simple as generating a guest crash dump is not the >>> >> >>> friendliest thing in the world to do to users. >>> >> >> >>> >> >> nmi is a fine name for something that corresponds to a real-life nmi >>> >> >> button (often labeled "NMI"). >>> >> > >>> >> > Agree. >>> >> >>> >> We could also introduce an alias mechanism for user friendly names, so >>> >> nmi could be used in addition of full path. Aliases could be useful >>> >> for device paths as well. >>> > >>> > Yes. Perhaps limited to the human monitor. >>> >>> I'd limit all debugging commands (including NMI) to the human monitor. >> >> Why? > > Do they have any real use in production environment? Also, we should > have the freedom to change the debugging facilities (for example, to > improve some internal implementation) as we want without regard to > compatibility to previous versions. For what it's worth, Lai (original poster) has been trying for many months to get inject-nmi into QMP, and I suspect he has a really good reason for his super-human persistence. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 2/2 V7] qemu,qmp: add inject-nmi qmp command
Avi Kivity writes: > On 04/08/2011 12:41 AM, Anthony Liguori wrote: >> >> And it's a good thing to have, but exposing this as the only API to >> do something as simple as generating a guest crash dump is not the >> friendliest thing in the world to do to users. > > nmi is a fine name for something that corresponds to a real-life nmi > button (often labeled "NMI"). Agree. > generate-crash-dump is a wrong name for something that doesn't > generate a crash dump (the guest may not be configured for it, or it > may fail to work). Or the OS uses the NMI button for something else. > I'd expect that to be host-side functionality. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] Native Linux KVM tool
Anthony Liguori writes: > On 04/03/2011 05:11 AM, Avi Kivity wrote: >> On 04/03/2011 12:59 PM, Pekka Enberg wrote: >>> Hi Avi, >>> >>> On Sun, Apr 3, 2011 at 11:23 AM, Avi Kivity wrote: >>> >> Note that this is a development prototype for the time being: >>> there's no >>> >> networking support and no graphics support, amongst other missing >>> >> essentials. >>> > >>> > Mind posting a roadmap? I would put smp support near the top. >>> This sort of >>> > thing has to be designed in, otherwise you wind up with a big >>> lock like >>> > qemu. >>> >>> What are the pain points with qemu at the moment? >> >> It's an ugly gooball. > > Because it solves a lot of very difficult problems. And the solutions emerged / evolved over a long time. Meanwhile, goals shifted. It wasn't designed as user space for KVM, it got shoehorned into that role (successfully). It has some solutions it should have left to other tools. For instance, it shouldn't be in the network configuration business. > You could drop all of the TCG support and it'd still be an ugly gooball. > > Supporting lots of different emulated hardware devices, live > migration, tons of different types of networking and image formats, > etc., all adds up over time. It does. Still, a fresh start could lead to a less ugly gooball. >>> SMP, networking, and simpler guest to host communication from shell >>> are most interesting missing features for me. >> >> If it is to be more than a toy, then Windows (really generic guest) >> support, manageability, live migration, hotplug, etc. are all >> crucial. > > I concur that SMP is probably one of those features you need to start > with if you're designing something from scratch. Certainly. Another one that doesn't like retrofitting is security. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 2/2 V7] qemu,qmp: add inject-nmi qmp command
[Note cc: Anthony]
"Daniel P. Berrange" writes:
> On Mon, Mar 07, 2011 at 05:46:28PM +0800, Lai Jiangshan wrote:
>> From: Lai Jiangshan
>> Date: Mon, 7 Mar 2011 17:05:15 +0800
>> Subject: [PATCH 2/2] qemu,qmp: add inject-nmi qmp command
>>
>> inject-nmi command injects an NMI on all CPUs of guest.
>> It is only supported for x86 guest currently, it will
>> returns "Unsupported" error for non-x86 guest.
>>
>> ---
>> hmp-commands.hx |2 +-
>> monitor.c | 18 +-
>> qmp-commands.hx | 29 +
>> 3 files changed, 47 insertions(+), 2 deletions(-)
>
> Does anyone have any feedback on this addition, or are all new
> QMP patch proposals blocked pending Anthony's QAPI work ?
That would be bad. Anthony, what's holding this back?
> We'd like to support it in libvirt and thus want it to be
> available in QMP, as well as HMP.
>
>> @@ -2566,6 +2566,22 @@ static void do_inject_nmi(Monitor *mon, const QDict
>> *qdict)
>> break;
>> }
>> }
>> +
>> +static int do_inject_nmi(Monitor *mon, const QDict *qdict, QObject
>> **ret_data)
>> +{
>> +CPUState *env;
>> +
>> +for (env = first_cpu; env != NULL; env = env->next_cpu)
>> +cpu_interrupt(env, CPU_INTERRUPT_NMI);
>> +
>> +return 0;
>> +}
>> +#else
>> +static int do_inject_nmi(Monitor *mon, const QDict *qdict, QObject
>> **ret_data)
>> +{
>> +qerror_report(QERR_UNSUPPORTED);
>> +return -1;
>> +}
>> #endif
>>
>
> Interesting that with HMP you need to specify a single CPU index, but
> with QMP it is injecting to all CPUs at once. Is there any compelling
> reason why we'd ever need the ability to only inject to a single CPU
> from an app developer POV ?
Quoting my own executive summary on this issue:
* Real hardware's NMI button injects all CPUs. This is the primary use
case.
* Lai said injecting a single CPU can be useful for debugging. Was
deemed acceptable as secondary use case.
Lai also pointed out that the human monitor's nmi command injects a
single CPU. That was dismissed as irrelevant for QMP.
* No other use cases have been presented.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB EHCI patch for 0.14.0?
Erik Rull writes: > Markus Armbruster wrote: >> erik.r...@rdsoftware.de writes: >> >>> Hi David, >>> >>> I did a second iteration and it looked way better, maybe my first attempts >>> were somehow buggy. >>> >>> First - please review your DPRINTF in the usb-ehci.c, there is a variable >>> "dev" undefined in line 504/505 when enabling the debugging defines at the >>> top of the file, the compiler complains there. >>> >>> I tested again with your hints, here my results: >>> >>> - using -device usb-host causes windows not to boot completely, using >>> -usbdevice host:auto:*.* is fine! >>> >>> - using -usbdevice tablet is better than using -device usb-tablet >> >> Better inhowfar? >> >> [...] > > It seems to work fine, the other option caused either windows not to > boot fully or the auto grabbing itself doesn't seem to work or the > mouse just didn't move. I experienced all of the three, but I haven't > noted down in which combinations - sorry. Please provide recipes to reproduce the bugs. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB EHCI patch for 0.14.0?
David Ahern writes: > On 03/11/11 08:39, Markus Armbruster wrote: >> erik.r...@rdsoftware.de writes: >> >>> Hi David, >>> >>> I did a second iteration and it looked way better, maybe my first attempts >>> were somehow buggy. >>> >>> First - please review your DPRINTF in the usb-ehci.c, there is a variable >>> "dev" undefined in line 504/505 when enabling the debugging defines at the >>> top of the file, the compiler complains there. >>> >>> I tested again with your hints, here my results: >>> >>> - using -device usb-host causes windows not to boot completely, using >>> -usbdevice host:auto:*.* is fine! >>> >>> - using -usbdevice tablet is better than using -device usb-tablet >> >> Better inhowfar? > > As I recall the USB code is the ignored step-child in qemu; it was never > properly/fully integrated into the qdev stuff. The paths appear to > differ in how usb devices are handled. I for one have always stuck with > the -usbdevice route. Please report any deficiencies so we can fix this. -usbdevice is around for backward compatibility only. There are already cases where -device provides you more control than -usbdevice. This feature gap will only grow. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB EHCI patch for 0.14.0?
erik.r...@rdsoftware.de writes: > Hi David, > > I did a second iteration and it looked way better, maybe my first attempts > were somehow buggy. > > First - please review your DPRINTF in the usb-ehci.c, there is a variable > "dev" undefined in line 504/505 when enabling the debugging defines at the > top of the file, the compiler complains there. > > I tested again with your hints, here my results: > > - using -device usb-host causes windows not to boot completely, using > -usbdevice host:auto:*.* is fine! > > - using -usbdevice tablet is better than using -device usb-tablet Better inhowfar? [...] -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH V6 3/4] qmp, nmi: convert do_inject_nmi() to QObject
Anthony Liguori writes: > On 02/25/2011 03:54 AM, Markus Armbruster wrote: >> Anthony Liguori writes: >> >> >>> On 02/24/2011 10:20 AM, Markus Armbruster wrote: >>> >>>> Anthony Liguori writes: >>>> >>>> >>>> >>>>> On 02/24/2011 02:33 AM, Markus Armbruster wrote: >>>>> >>>>> >>>>>> Anthony Liguoriwrites: >>>>>> >> [...] >> >>>>>>> Please describe all expected errors. >>>>>>> >>>>>>> >>>>>>> >>>>>> Quoting qmp-commands.hx: >>>>>> >>>>>>3. Errors, in special, are not documented. Applications should >>>>>> NOT check >>>>>> for specific errors classes or data (it's strongly recommended >>>>>> to only >>>>>> check for the "error" key) >>>>>> >>>>>> Indeed, not a single error is documented there. This is intentional. >>>>>> >>>>>> >>>>>> >>>>> Yeah, but we're not 0.14 anymore and for 0.15, we need to document >>>>> errors. If you are suggesting I send a patch to remove that section, >>>>> I'm more than happy to. >>>>> >>>>> >>>> Two separate issues here: 1. Are we ready to commit to the current >>>> design of errors, and 2. Is it fair to reject Lai's patch now because he >>>> doesn't document his errors. >>>> >>>> I'm not commenting on 1. here. >>>> >>>> Regarding 2.: rejecting a patch because it doesn't document an aspect >>>> that current master intentionally leaves undocumented is not how you >>>> treat contributors. At least not if you want any other than certified >>>> masochists who enjoy pain, and professionals who get adequately >>>> compensated for it. >>>> >>>> Lead by example, not by fiat. >>>> >>>> >>> http://repo.or.cz/w/qemu/aliguori.git/blob/refs/heads/glib:/qmp-schema.json >>> >>> I am in the process of documenting the errors of every command. It's >>> a royal pain but I'm going to document everything we have right now. >>> It's actually the last bit of work I need to finish before sending >>> QAPI out. >>> >>> So for new commands being added, it would be hugely helpful for the >>> authors to document the errors such that I don't have to reverse >>> engineer all of the possible error conditions. >>> >> The moment this lands in master, you can begin to demand error >> descriptions from contributors. Until then, I'll NAK error descriptions >> in qmp-commands.txt. We left them undocumented there for good reasons: >> > > No, it was always a bad reason. Good documentation is necessary to > build good commands. Errors are a huge part of the semantics of a > command. We cannot properly assess a command unless it's behavior in > error conditions is well defined. The decision to declare QMP stable was made at the KVM Forum after quite some deliberation. We explictly excluded errors. We didn't do that because errors are not worthy of specification (that would be silly). We did it because there was too much unhappiness about the current state of errors to close the debate on them at that time. Luiz mindfully documented that decision in qmp-commands.txt: 3. Errors, in special, are not documented. Applications should NOT check for specific errors classes or data (it's strongly recommended to only check for the "error" key) Nobody denies that errors need to be specified, and this clause needs to go. But I must insist on proper review. No sneaking in of error specification through the commit backdoor, please. >>>>>> Once we have an error design in place that has a reasonable hope to >>>>>> stand the test of time, and have errors documented for at least some of >>>>>> the commands here, we can start to require proper error documentation >>>>>> for new commands. But not now. >>>>>> >> I won't NAK non-normative error descriptions, say in commit messages, or >> in comments. And I won't object to you asking for them. But I feel you >> really shouldn't ma
Re: [Qemu-devel] [PATCH V6 3/4] qmp, nmi: convert do_inject_nmi() to QObject
Anthony Liguori writes: > On 02/24/2011 10:20 AM, Markus Armbruster wrote: >> Anthony Liguori writes: >> >> >>> On 02/24/2011 02:33 AM, Markus Armbruster wrote: >>> >>>> Anthony Liguori writes: [...] >>>>> Please describe all expected errors. >>>>> >>>>> >>>> Quoting qmp-commands.hx: >>>> >>>> 3. Errors, in special, are not documented. Applications should NOT >>>> check >>>> for specific errors classes or data (it's strongly recommended to >>>> only >>>> check for the "error" key) >>>> >>>> Indeed, not a single error is documented there. This is intentional. >>>> >>>> >>> Yeah, but we're not 0.14 anymore and for 0.15, we need to document >>> errors. If you are suggesting I send a patch to remove that section, >>> I'm more than happy to. >>> >> Two separate issues here: 1. Are we ready to commit to the current >> design of errors, and 2. Is it fair to reject Lai's patch now because he >> doesn't document his errors. >> >> I'm not commenting on 1. here. >> >> Regarding 2.: rejecting a patch because it doesn't document an aspect >> that current master intentionally leaves undocumented is not how you >> treat contributors. At least not if you want any other than certified >> masochists who enjoy pain, and professionals who get adequately >> compensated for it. >> >> Lead by example, not by fiat. >> > > http://repo.or.cz/w/qemu/aliguori.git/blob/refs/heads/glib:/qmp-schema.json > > I am in the process of documenting the errors of every command. It's > a royal pain but I'm going to document everything we have right now. > It's actually the last bit of work I need to finish before sending > QAPI out. > > So for new commands being added, it would be hugely helpful for the > authors to document the errors such that I don't have to reverse > engineer all of the possible error conditions. The moment this lands in master, you can begin to demand error descriptions from contributors. Until then, I'll NAK error descriptions in qmp-commands.txt. We left them undocumented there for good reasons: >>>> Once we have an error design in place that has a reasonable hope to >>>> stand the test of time, and have errors documented for at least some of >>>> the commands here, we can start to require proper error documentation >>>> for new commands. But not now. I won't NAK non-normative error descriptions, say in commit messages, or in comments. And I won't object to you asking for them. But I feel you really shouldn't make it a condition for committing patches. Especially not for simple patches that have been on list for months. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH V6 3/4] qmp, nmi: convert do_inject_nmi() to QObject
Anthony Liguori writes:
> On 02/24/2011 02:33 AM, Markus Armbruster wrote:
>> Anthony Liguori writes:
>>
>>
>>> On 01/27/2011 02:20 AM, Lai Jiangshan wrote:
>>>
>>>> Make we can inject NMI via qemu-monitor-protocol.
>>>> We use "inject-nmi" for the qmp command name, the meaning is clearer.
>>>>
>>>> Signed-off-by: Lai Jiangshan
>>>> ---
>>>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>>>> index ec1a4db..e763bf9 100644
>>>> --- a/hmp-commands.hx
>>>> +++ b/hmp-commands.hx
>>>> @@ -725,7 +725,8 @@ ETEXI
>>>>.params = "[cpu]",
>>>>.help = "Inject an NMI on all CPUs if no argument is
>>>> given, "
>>>> "otherwise inject it on the specified CPU",
>>>> -.mhandler.cmd = do_inject_nmi,
>>>> +.user_print = monitor_user_noop,
>>>> +.mhandler.cmd_new = do_inject_nmi,
>>>>},
>>>>#endif
>>>>STEXI
>>>> diff --git a/monitor.c b/monitor.c
>>>> index 387b020..1b1c0ba 100644
>>>> --- a/monitor.c
>>>> +++ b/monitor.c
>>>> @@ -2542,7 +2542,7 @@ static void do_wav_capture(Monitor *mon, const QDict
>>>> *qdict)
>>>>#endif
>>>>
>>>>#if defined(TARGET_I386)
>>>> -static void do_inject_nmi(Monitor *mon, const QDict *qdict)
>>>> +static int do_inject_nmi(Monitor *mon, const QDict *qdict, QObject
>>>> **ret_data)
>>>>{
>>>>CPUState *env;
>>>>int cpu_index;
>>>> @@ -2550,7 +2550,7 @@ static void do_inject_nmi(Monitor *mon, const QDict
>>>> *qdict)
>>>>if (!qdict_haskey(qdict, "cpu-index")) {
>>>>for (env = first_cpu; env != NULL; env = env->next_cpu)
>>>>cpu_interrupt(env, CPU_INTERRUPT_NMI);
>>>> -return;
>>>> +return 0;
>>>>}
>>>>
>>>>cpu_index = qdict_get_int(qdict, "cpu-index");
>>>> @@ -2560,8 +2560,10 @@ static void do_inject_nmi(Monitor *mon, const QDict
>>>> *qdict)
>>>>kvm_inject_interrupt(env, CPU_INTERRUPT_NMI);
>>>>else
>>>>cpu_interrupt(env, CPU_INTERRUPT_NMI);
>>>> -break;
>>>> +return 0;
>>>>}
>>>> +
>>>> +return -1;
>>>>}
>>>>#endif
>>>>
>>>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>>>> index 56c4d8b..a887dd5 100644
>>>> --- a/qmp-commands.hx
>>>> +++ b/qmp-commands.hx
>>>> @@ -429,6 +429,34 @@ Example:
>>>>
>>>>EQMP
>>>>
>>>> +#if defined(TARGET_I386)
>>>> +{
>>>> +.name = "inject-nmi",
>>>> +.args_type = "cpu-index:i?",
>>>> +.params = "[cpu]",
>>>> +.help = "Inject an NMI on all CPUs if no argument is given,
>>>> "
>>>> + "otherwise inject it on the specified CPU",
>>>> +.user_print = monitor_user_noop,
>>>> +.mhandler.cmd_new = do_inject_nmi,
>>>> +},
>>>> +#endif
>>>> +SQMP
>>>> +inject-nmi
>>>> +--
>>>> +
>>>> +Inject an NMI on all CPUs or the given CPU (x86 only).
>>>> +
>>>> +Arguments:
>>>> +
>>>> +- "cpu-index": the index of the CPU to be injected NMI (json-int,
>>>> optional)
>>>> +
>>>> +Example:
>>>> +
>>>> +-> { "execute": "inject-nmi", "arguments": { "cpu-index": 0 } }
>>>> +<- { "return": {} }
>>>>
>>>>
>>> Please describe all expected errors.
>>>
>> Quoting qmp-commands.hx:
>>
>> 3. Errors, in special, are not documented. Applications should NOT check
>> for specific errors classes or data (it's strongly recommended to
>> only
>> check for the "error" key)
>>
>> Indeed, not a single error is documented there. This i
Re: [Qemu-devel] [PATCH V6 3/4] qmp, nmi: convert do_inject_nmi() to QObject
Anthony Liguori writes:
> On 01/27/2011 02:20 AM, Lai Jiangshan wrote:
>> Make we can inject NMI via qemu-monitor-protocol.
>> We use "inject-nmi" for the qmp command name, the meaning is clearer.
>>
>> Signed-off-by: Lai Jiangshan
>> ---
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index ec1a4db..e763bf9 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -725,7 +725,8 @@ ETEXI
>> .params = "[cpu]",
>> .help = "Inject an NMI on all CPUs if no argument is given, "
>> "otherwise inject it on the specified CPU",
>> -.mhandler.cmd = do_inject_nmi,
>> +.user_print = monitor_user_noop,
>> +.mhandler.cmd_new = do_inject_nmi,
>> },
>> #endif
>> STEXI
>> diff --git a/monitor.c b/monitor.c
>> index 387b020..1b1c0ba 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -2542,7 +2542,7 @@ static void do_wav_capture(Monitor *mon, const QDict
>> *qdict)
>> #endif
>>
>> #if defined(TARGET_I386)
>> -static void do_inject_nmi(Monitor *mon, const QDict *qdict)
>> +static int do_inject_nmi(Monitor *mon, const QDict *qdict, QObject
>> **ret_data)
>> {
>> CPUState *env;
>> int cpu_index;
>> @@ -2550,7 +2550,7 @@ static void do_inject_nmi(Monitor *mon, const QDict
>> *qdict)
>> if (!qdict_haskey(qdict, "cpu-index")) {
>> for (env = first_cpu; env != NULL; env = env->next_cpu)
>> cpu_interrupt(env, CPU_INTERRUPT_NMI);
>> -return;
>> +return 0;
>> }
>>
>> cpu_index = qdict_get_int(qdict, "cpu-index");
>> @@ -2560,8 +2560,10 @@ static void do_inject_nmi(Monitor *mon, const QDict
>> *qdict)
>> kvm_inject_interrupt(env, CPU_INTERRUPT_NMI);
>> else
>> cpu_interrupt(env, CPU_INTERRUPT_NMI);
>> -break;
>> +return 0;
>> }
>> +
>> +return -1;
>> }
>> #endif
>>
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index 56c4d8b..a887dd5 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -429,6 +429,34 @@ Example:
>>
>> EQMP
>>
>> +#if defined(TARGET_I386)
>> +{
>> +.name = "inject-nmi",
>> +.args_type = "cpu-index:i?",
>> +.params = "[cpu]",
>> +.help = "Inject an NMI on all CPUs if no argument is given, "
>> + "otherwise inject it on the specified CPU",
>> +.user_print = monitor_user_noop,
>> +.mhandler.cmd_new = do_inject_nmi,
>> +},
>> +#endif
>> +SQMP
>> +inject-nmi
>> +--
>> +
>> +Inject an NMI on all CPUs or the given CPU (x86 only).
>> +
>> +Arguments:
>> +
>> +- "cpu-index": the index of the CPU to be injected NMI (json-int, optional)
>> +
>> +Example:
>> +
>> +-> { "execute": "inject-nmi", "arguments": { "cpu-index": 0 } }
>> +<- { "return": {} }
>>
>
> Please describe all expected errors.
Quoting qmp-commands.hx:
3. Errors, in special, are not documented. Applications should NOT check
for specific errors classes or data (it's strongly recommended to only
check for the "error" key)
Indeed, not a single error is documented there. This is intentional.
Once we have an error design in place that has a reasonable hope to
stand the test of time, and have errors documented for at least some of
the commands here, we can start to require proper error documentation
for new commands. But not now.
> Don't hide this command for
> !defined(TARGET_I386), instead have it throw an error in the
> implementation.
Works for me.
> Don't have commands that multiple behavior based on the presence or
> absence of arguments. Make it take a list of cpus if you want the
> ability to inject the NMI to more than one CPU.
Having optional arguments is fine. It's good taste to give them
"default semantics", i.e. "no argument" is shorthand for one specific
argument value.
Luiz already pointed to the thread where we discussed this command
before. Executive summary:
* Real hardware's NMI button injects all CPUs. This is the primary use
case.
* Lai said injecting a single CPU can be useful for debugging. Was
deemed acceptable as secondary use case.
Lai also pointed out that the human monitor's nmi command injects a
single CPU. That was dismissed as irrelevant for QMP.
* No other use cases have been presented.
Therefore, the "list of CPUs" idea was shot down as overly general.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] Re: [PATCH V6 1/4 resend] nmi: convert cpu_index to cpu-index
Luiz Capitulino writes: > On Mon, 21 Feb 2011 09:37:57 +0800 > Lai Jiangshan wrote: > >> Hi, Luiz Capitulino >> >> Any problem? > > Sorry for the delay. Looks good in general to me know, there's only one > small problem and it's the error message: > > (qemu) nmi 100 > Parameter 'cpu-index' expects a CPU number > (qemu) > > I would expect that kind of error message when no CPU number is > provided, but in the case above the CPU number is provided but it > happens to be invalid. Why? This is not Lai Jiangshan's fault. It's what QERR_INVALID_PARAMETER_VALUE reports. The current design of QError makes it hard to do better. "expects a valid CPU number" could be done, if you think that's better. > By the way, please add an introductory email with proper changelog > when submitting series/patches, so that it's easier to review. Also make sure the parts are threaded together properly with In-Reply-To and References headers, because that helps e-mail readers to keep the parts together. Lack of threading is annoying, and annoying reviewers intentionally would be rude :) Suggested workflow: git-format-patch --cover-letter -ns ... look over patch files, edit the cover letter to taste git-send-email --to qemu-de...@nongnu.org 0*.patch -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call agenda for Feb 15
Anthony Liguori writes: > On 02/14/2011 11:56 AM, Chris Wright wrote: >> Please send in any agenda items you are interested in covering. >> > > - QAPI and QMP events > - qdev future Do we need a qdev tree and a maintainer for it? > > I don't really have a coherent plan for the second one yet so let's > just discuss this as time permits. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] KVM call minutes for Feb 8
Anthony Liguori writes: > On 02/09/2011 02:01 AM, Markus Armbruster wrote: >> Anthony Liguori writes: [...] >>> We need to unify the property model. We have QemuOpts, qdev >>> properties, and QObject which basically reinvents variant typing three >>> different ways. >>> >> Make it four: QEMUOptionParameter. >> >> Now let me make it three again. Unlike the others, a qdev property >> describes a perfectly ordinary, non-variant struct member. It's poor >> man's reflection, not a variant type. >> > > Except that construction of a device requires initialization from an > array of variants (which is then type checked). The way we store the > variants is lossy because we convert back and forth to a string. Yes, there's overlap, but no, a qdev property isn't yet another variant type scheme. Exhibit A of the defense: qdev uses QemuOpts for variant types. Let me elaborate. qdev_device_add() uses QemuOpts as map from name to variant type value, uses the name to look up the property, then uses property methods to stuff the variant value it got from QemuOpts into the (non-variant) struct member described by the property. I figure QemuOpts was adopted for this purpose because it was already in use with command line and human monitor. With QMP added to the mix, there's friction: QMP uses QDict, not QemuOpts. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
