[Qemu-devel] Re: [PATCH v4 03/10] x86: Extend validity of cpu_is_bsp
On Thu, Mar 04, 2010 at 12:34:22AM +0100, Jan Kiszka wrote: > Gleb Natapov wrote: > > On Mon, Mar 01, 2010 at 06:17:22PM +0100, Jan Kiszka wrote: > >> As we hard-wire the BSP to CPU 0 anyway and cpuid_apic_id equals > >> cpu_index, cpu_is_bsp can also be based on the latter directly. This > >> will help an early user of it: KVM while initializing mp_state. > >> > >> Signed-off-by: Jan Kiszka > >> --- > >> hw/pc.c |3 ++- > >> 1 files changed, 2 insertions(+), 1 deletions(-) > >> > >> diff --git a/hw/pc.c b/hw/pc.c > >> index b90a79e..58c32ea 100644 > >> --- a/hw/pc.c > >> +++ b/hw/pc.c > >> @@ -767,7 +767,8 @@ static void pc_init_ne2k_isa(NICInfo *nd) > >> > >> int cpu_is_bsp(CPUState *env) > >> { > >> -return env->cpuid_apic_id == 0; > >> +/* We hard-wire the BSP to the first CPU. */ > >> +return env->cpu_index == 0; > >> } > > We should not assume that. The function was written like that > > specifically so the code around it will not rely on this assumption. > > Now you change that specifically to write code that will do incorrect > > assumptions. I don't see the logic here. > > The logic is that we do not support any other mapping yet - with or > without this change. Without it, we complicate the APIC initialization > for (so far) no good reason. Once we want to support different BSP > assignments, we need to go through the code and rework some parts anyway. > As far as I remember the only part that was missing was a command line to specify apic IDs for each CPU and what CPU is BSP. The code was ready otherwise. I's very sad if this was broken by other modifications. But changes like that actually pushes us back from our goal. Why not rework code so it will work with correct cpu_is_bsp() function instead of introducing this hack? -- Gleb.
Re: [Qemu-devel] qemu on Solaris
Quoting Jonathan Kalbfeld, who wrote the following on Wed, 3 Mar 2010: Hey Seth, Do you know if it builds on Sparc at all? On Oracle Solaris ;) Yes, with the below caveats, it does build and operate on SPARC as well :). --S jonathan On Wed, Mar 3, 2010 at 6:24 PM, Seth G wrote: Hi, I just sent a fair amount of time trying to figure out problems with the qemu build and resulting binary. The first problem (which is no longer a problem due to the shell OpenSolaris uses) was that `sh' was used in rules.mak to perform commands -- I had a /bin/sh that was not bash-compatible. That's a minor problem. The more-serious problem was that once qemu was built, it would die almost immediately, complaining about smbus-eeprom not being an I2C device. That problem appears to be due to an issue with gcc's use of the Solaris native linker (/usr/ccs/bin/ld) -- the linker is not including all .o's in the .a's, even though the --whole-archive switch was passed to it. The linker team is looking into that problem right now, but in the meantime, manually performing the link step by specifying all the required .o's yields a usable qemu executable. Thanks, --S -- -- Jonathan Kalbfeld ThoughtWave Technologies LLC www.thoughtwave.com LA: +1 818.306.5353 NY: +1 212.202.0698 Direct: +1 818 495 2139 FAX: +1 818 495 2140 Learn UNIX For Free at unixlessons.com
Re: [Qemu-devel] qemu on Solaris
Hey Seth, Do you know if it builds on Sparc at all? On Oracle Solaris ;) jonathan On Wed, Mar 3, 2010 at 6:24 PM, Seth G wrote: > Hi, > > I just sent a fair amount of time trying to figure out problems > with the qemu build and resulting binary. The first problem (which > is no longer a problem due to the shell OpenSolaris uses) was that > `sh' was used in rules.mak to perform commands -- I had a /bin/sh > that was not bash-compatible. That's a minor problem. The > more-serious problem was that once qemu was built, it would die almost > immediately, complaining about smbus-eeprom not being an I2C device. > That problem appears to be due to an issue with gcc's use of the > Solaris native linker (/usr/ccs/bin/ld) -- the linker is not including > all .o's in the .a's, even though the --whole-archive switch was passed > to it. The linker team is looking into that problem right now, but in > the meantime, manually performing the link step by specifying all the > required .o's yields a usable qemu executable. > > Thanks, > --S > > > > -- -- Jonathan Kalbfeld ThoughtWave Technologies LLC www.thoughtwave.com LA: +1 818.306.5353 NY: +1 212.202.0698 Direct: +1 818 495 2139 FAX: +1 818 495 2140 Learn UNIX For Free at unixlessons.com
[Qemu-devel] qemu on Solaris
Hi, I just sent a fair amount of time trying to figure out problems with the qemu build and resulting binary. The first problem (which is no longer a problem due to the shell OpenSolaris uses) was that `sh' was used in rules.mak to perform commands -- I had a /bin/sh that was not bash-compatible. That's a minor problem. The more-serious problem was that once qemu was built, it would die almost immediately, complaining about smbus-eeprom not being an I2C device. That problem appears to be due to an issue with gcc's use of the Solaris native linker (/usr/ccs/bin/ld) -- the linker is not including all .o's in the .a's, even though the --whole-archive switch was passed to it. The linker team is looking into that problem right now, but in the meantime, manually performing the link step by specifying all the required .o's yields a usable qemu executable. Thanks, --S
[Qemu-devel] Re: [PATCH 2/4] KVM: Rework VCPU state writeback API
On Thu, Mar 04, 2010 at 01:21:12AM -0300, Marcelo Tosatti wrote: > The regression seems to be caused by seabios commit d7e998f. Kevin, the > failure can be seen on the attached screenshot, which happens on the > first reboot of WinXP 32 installation (after copying files etc). Sorry - I also noticed a bug in that commit recently. I pushed the fix I had in my local tree. -Kevin
[Qemu-devel] Re: [PATCH 2/4] KVM: Rework VCPU state writeback API
On Tue, Mar 02, 2010 at 11:29:10PM -0300, Marcelo Tosatti wrote: > On Tue, Mar 02, 2010 at 05:31:09PM +0100, Jan Kiszka wrote: > > Marcelo Tosatti wrote: > > > On Tue, Mar 02, 2010 at 09:00:04AM +0100, Jan Kiszka wrote: > > >> Marcelo Tosatti wrote: > > >>> On Mon, Mar 01, 2010 at 07:10:30PM +0100, Jan Kiszka wrote: > > This grand cleanup drops all reset and vmsave/load related > > synchronization points in favor of four(!) generic hooks: > > > > - cpu_synchronize_all_states in qemu_savevm_state_complete > > (initial sync from kernel before vmsave) > > - cpu_synchronize_all_post_init in qemu_loadvm_state > > (writeback after vmload) > > - cpu_synchronize_all_post_init in main after machine init > > - cpu_synchronize_all_post_reset in qemu_system_reset > > (writeback after system reset) > > > > These writeback points + the existing one of VCPU exec after > > cpu_synchronize_state map on three levels of writeback: > > > > - KVM_PUT_RUNTIME_STATE (during runtime, other VCPUs continue to run) > > - KVM_PUT_RESET_STATE (on synchronous system reset, all VCPUs > > stopped) > > - KVM_PUT_FULL_STATE(on init or vmload, all VCPUs stopped as well) > > > > This level is passed to the arch-specific VCPU state writing function > > that will decide which concrete substates need to be written. That way, > > no writer of load, save or reset functions that interact with in-kernel > > KVM states will ever have to worry about synchronization again. That > > also means that a lot of reasons for races, segfaults and deadlocks are > > eliminated. > > > > cpu_synchronize_state remains untouched, just as Anthony suggested. We > > continue to need it before reading or writing of VCPU states that are > > also tracked by in-kernel KVM subsystems. > > > > Consequently, this patch removes many cpu_synchronize_state calls that > > are now redundant, just like remaining explicit register syncs. > > > > Signed-off-by: Jan Kiszka > > >>> Jan, > > >>> > > >>> This patch breaks system reset of WinXP.32 install (more easily > > >>> reproducible without iothread enabled). > > >>> > > >>> Screenshot attached. > > >>> > > >> Strange - no issues with qemu-kvm? Any special command line switch? /me > > >> goes scrounging for some installation XP32 CD in the meantime... > > > > > > No issues with qemu-kvm. Could not spot anything obvious. > > > > > > > And, of course, my WinXP installation did not trigger any reset issue, > > even in non-iothreaded mode. :( The regression seems to be caused by seabios commit d7e998f. Kevin, the failure can be seen on the attached screenshot, which happens on the first reboot of WinXP 32 installation (after copying files etc). <>
Re: [Qemu-devel] Re: EHCI support in QEMU
On 03/03/2010 04:47 PM, Jan Kiszka wrote: > > Thanks for your work, David and Niels! I assume that David based this on > Niels' patch, so there is nothing to be merged? David's version built > for me, so I pushed > > git://git.kiszka.org/qemu.git ehci > > So far it's supposed to be a reference for anyone interested in this > topic, willing to test or even to hack on it. I will collect and merge > improvements as they are sent to the list. > > Jan > Yes, I took Niels post as a start. Addressed a few white space issues, made some of the initializations parallel the uhci code and removed the mem_base from the read/write functions (causing segfaults due to outbounds referencing -- addr > MMIO_SIZE -- and do not appear to be needed). David
Re: [Qemu-devel] Re: EHCI support in QEMU
David S. Ahern wrote: > On 03/03/2010 04:21 AM, Niels de Vos wrote: >> On Tue, Mar 2, 2010 at 10:48 AM, Kevin Wolf wrote: >>> Am 01.03.2010 22:17, schrieb Jan Kiszka: Niels de Vos wrote: > If someone is interested in this partially ported patch, I'm happy to > share, but it will at least need some attention to make it compile. > After that, lots of tests need to be done and probably quite some > bugfixes are required. I'm happy to assist, but do not have a lot of > time to spare on this hobby project. On the occasion that it is something > more solid and starting to do something, I will of course inform this list > again. OK, to keep this heavy ball rolling, I would suggest posting your patch. Either it's already in a good shape to get it merged as experimental feature. Or I will pick it up in git tree, collect patches as they fly in, and will keep on pushing it upstream. I can't promise spending much time on hacking, but integration work, basic testing, and some more or less helpful comments should be feasible. >>> Yes, please post it. I won't promise anything either, but maybe I can >>> find some time to help a bit. Anyway, I'd love to see EHCI in qemu. >> Okay, here it is. Please note that the patch is very raw, does not >> compile and even basic things like indention is broken at the moment. >> Please refer to >> http://lists.gnu.org/archive/html/qemu-devel/2008-10/msg01326.html for >> the original author, nothing seriously of this patch is done by me. >> >> Kind regards and thanks for your help, >> Niels > > FWIW: the attached compiles and qemu does not die (patch created against > qemu-kvm.git but applies to qemu.git as well). For now I disabled uhci > so that the device is attached to the ehci bus. I can attach a usb key, > but lsusb in the guest (fedora core 12) does not show it. > Thanks for your work, David and Niels! I assume that David based this on Niels' patch, so there is nothing to be merged? David's version built for me, so I pushed git://git.kiszka.org/qemu.git ehci So far it's supposed to be a reference for anyone interested in this topic, willing to test or even to hack on it. I will collect and merge improvements as they are sent to the list. Jan signature.asc Description: OpenPGP digital signature
[Qemu-devel] Re: [PATCH v4 03/10] x86: Extend validity of cpu_is_bsp
Gleb Natapov wrote: > On Mon, Mar 01, 2010 at 06:17:22PM +0100, Jan Kiszka wrote: >> As we hard-wire the BSP to CPU 0 anyway and cpuid_apic_id equals >> cpu_index, cpu_is_bsp can also be based on the latter directly. This >> will help an early user of it: KVM while initializing mp_state. >> >> Signed-off-by: Jan Kiszka >> --- >> hw/pc.c |3 ++- >> 1 files changed, 2 insertions(+), 1 deletions(-) >> >> diff --git a/hw/pc.c b/hw/pc.c >> index b90a79e..58c32ea 100644 >> --- a/hw/pc.c >> +++ b/hw/pc.c >> @@ -767,7 +767,8 @@ static void pc_init_ne2k_isa(NICInfo *nd) >> >> int cpu_is_bsp(CPUState *env) >> { >> -return env->cpuid_apic_id == 0; >> +/* We hard-wire the BSP to the first CPU. */ >> +return env->cpu_index == 0; >> } > We should not assume that. The function was written like that > specifically so the code around it will not rely on this assumption. > Now you change that specifically to write code that will do incorrect > assumptions. I don't see the logic here. The logic is that we do not support any other mapping yet - with or without this change. Without it, we complicate the APIC initialization for (so far) no good reason. Once we want to support different BSP assignments, we need to go through the code and rework some parts anyway. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] tcg: fix build on 32-bit hppa, ppc and sparc hosts
On Mon, Feb 22, 2010 at 05:35:16PM +0100, Stefan Weil wrote: > Maybe my comment applies also to the change in > tcg/hppa/tcg-target.c, but I tested only ppc > (using cross compilation). HPPA host support is 32-bit only for the time being, as there isn't even a 64-bit userspace for HPPA, yet, and that's unlikely to change soon. -- Cheers, Stuart Brady
Re: [Qemu-devel] Intel PXA270 System-on-chip emulation in QEMU
On 3 March 2010 18:24, Taimoor Mirza wrote: > Can you tell me name of any platform which is emulated in QEMU and I can run > my USB device application on it? I mean I want to haveĀ board(emulated in > QEMU) behaving as a mass storage device (running mass storage application). The N800 and N810 can do this (-M n800, -M n810), but it isn't easy to get a guest linux set up for these devices, so you're probably best off doing your testing on a PC using GadgetFS and the Dummy HCD driver. If you really want to test using -M n800, here's some guide: http://marcin.juszkiewicz.com.pl/2008/04/11/nokia-n800-emulation/ (and some of the following post blogs). Cheers
Re: [Qemu-devel] [PATCH 04/17] virtio-9p: Implement P9_TSTAT
On Wed, 3 Mar 2010, Anthony Liguori wrote: > This get the mount to work on the guest > > [ki...@linux.vnet.ibm.com: malloc to qemu_malloc conversion] > > Signed-off-by: Anthony Liguori > Signed-off-by: Gautham R Shenoy > Signed-off-by: Aneesh Kumar K.V > --- > hw/virtio-9p-local.c |7 ++ > hw/virtio-9p.c | 169 > +- > 2 files changed, 174 insertions(+), 2 deletions(-) > > diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c > index 204437c..9752f76 100644 > --- a/hw/virtio-9p-local.c > +++ b/hw/virtio-9p-local.c > @@ -72,9 +72,16 @@ static int local_setuid(void *opaque, uid_t uid) > return 0; > } > > +static ssize_t local_readlink(void *opaque, const char *path, > + char *buf, size_t bufsz) > +{ > +return readlink(rpath(path), buf, bufsz); > +} > + > static V9fsPosixFileOperations ops = { > .lstat = local_lstat, > .setuid = local_setuid, > +.readlink = local_readlink, > }; > > V9fsPosixFileOperations *virtio_9p_init_local(const char *path) > diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c > index c63ac80..10bcd89 100644 > --- a/hw/virtio-9p.c > +++ b/hw/virtio-9p.c > @@ -102,6 +102,21 @@ static int posix_setuid(V9fsState *s, uid_t uid) > return s->ops->setuid(s->ops->opaque, uid); > } > > +static ssize_t posix_readlink(V9fsState *s, V9fsString *path, V9fsString > *buf) > +{ > +ssize_t len; > + > +buf->data = qemu_malloc(1024); > + > +len = s->ops->readlink(s->ops->opaque, path->data, buf->data, 1024 - 1); > +if (len > -1) { > + buf->size = len; > + buf->data[len] = 0; > +} > + > +return len; > +} > + > static void v9fs_string_free(V9fsString *str) > { > free(str->data); Should be qemu_free, no? [..snip..] -- mailto:av1...@comtv.ru
[Qemu-devel] Re: [PATCHv3 19/20] eepro100: Remove C++ comments
On Wed, Mar 03, 2010 at 08:15:09PM +0100, Stefan Weil wrote: > Michael S. Tsirkin schrieb: > > On Tue, Mar 02, 2010 at 10:37:59PM +0100, Stefan Weil wrote: > >> C++ comments are unwanted, so this is fixed here. > >> > >> * Replace C++ comments by C comments. > >> * Put code which was deactivated by a C++ comment in #if 0...#endif. > >> > >> Signed-off-by: Stefan Weil > > > > It would be nice to add some documentation to commented > > blocks, explaining why we still keep them around > > and what happens if they are uncommented. > > > Typically, later patches either use this deactivated code (maybe after > some modifications), or they remove it completely. More documentation > would help of course - I'll try my best :-) > > The purpose of this patch was only elimination of C++ comments by formally > transforming them to either C comments or by using the preprocessor #if. Yes, I understand that.
[Qemu-devel] Re: [PATCHv3 19/20] eepro100: Remove C++ comments
Michael S. Tsirkin schrieb: > On Tue, Mar 02, 2010 at 10:37:59PM +0100, Stefan Weil wrote: >> C++ comments are unwanted, so this is fixed here. >> >> * Replace C++ comments by C comments. >> * Put code which was deactivated by a C++ comment in #if 0...#endif. >> >> Signed-off-by: Stefan Weil > > It would be nice to add some documentation to commented > blocks, explaining why we still keep them around > and what happens if they are uncommented. Typically, later patches either use this deactivated code (maybe after some modifications), or they remove it completely. More documentation would help of course - I'll try my best :-) The purpose of this patch was only elimination of C++ comments by formally transforming them to either C comments or by using the preprocessor #if.
[Qemu-devel] [PATCH 15/17] virtio-9p: Use little endian format on virtio
From: Aneesh Kumar K.V We need to use platform independent data format as part of protocol data. 9P uses little endian format on wire Signed-off-by: Aneesh Kumar K.V --- hw/virtio-9p.c | 34 +++--- 1 files changed, 23 insertions(+), 11 deletions(-) diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c index 97fcb8a..65f5827 100644 --- a/hw/virtio-9p.c +++ b/hw/virtio-9p.c @@ -425,23 +425,32 @@ static size_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...) for (i = 0; fmt[i]; i++) { switch (fmt[i]) { case 'b': { - int8_t *valp = va_arg(ap, int8_t *); + uint8_t *valp = va_arg(ap, uint8_t *); offset += pdu_unpack(valp, pdu, offset, sizeof(*valp)); break; } case 'w': { - int16_t *valp = va_arg(ap, int16_t *); - offset += pdu_unpack(valp, pdu, offset, sizeof(*valp)); + uint16_t val, *valp; + valp = va_arg(ap, uint16_t *); + val = le16_to_cpupu(valp); + offset += pdu_unpack(&val, pdu, offset, sizeof(val)); + *valp = val; break; } case 'd': { - int32_t *valp = va_arg(ap, int32_t *); - offset += pdu_unpack(valp, pdu, offset, sizeof(*valp)); + uint32_t val, *valp; + valp = va_arg(ap, uint32_t *); + val = le32_to_cpupu(valp); + offset += pdu_unpack(&val, pdu, offset, sizeof(val)); + *valp = val; break; } case 'q': { - int64_t *valp = va_arg(ap, int64_t *); - offset += pdu_unpack(valp, pdu, offset, sizeof(*valp)); + uint64_t val, *valp; + valp = va_arg(ap, uint64_t *); + val = le64_to_cpup(valp); + offset += pdu_unpack(&val, pdu, offset, sizeof(val)); + *valp = val; break; } case 'v': { @@ -497,22 +506,25 @@ static size_t pdu_marshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...) for (i = 0; fmt[i]; i++) { switch (fmt[i]) { case 'b': { - int8_t val = va_arg(ap, int); + uint8_t val = va_arg(ap, int); offset += pdu_pack(pdu, offset, &val, sizeof(val)); break; } case 'w': { - int16_t val = va_arg(ap, int); + uint16_t val; + cpu_to_le16w(&val, va_arg(ap, int)); offset += pdu_pack(pdu, offset, &val, sizeof(val)); break; } case 'd': { - int32_t val = va_arg(ap, int); + uint32_t val; + cpu_to_le32w(&val, va_arg(ap, uint32_t)); offset += pdu_pack(pdu, offset, &val, sizeof(val)); break; } case 'q': { - int64_t val = va_arg(ap, int64_t); + uint64_t val; + cpu_to_le64w(&val, va_arg(ap, uint64_t)); offset += pdu_pack(pdu, offset, &val, sizeof(val)); break; } -- 1.6.5.2
[Qemu-devel] [PATCH 12/17] virtio-9p: Implement P9_TREMOVE
This gets file deletion to work [mo...@in.ibm.com: Fix truncate to use the relative path] Signed-off-by: Anthony Liguori Signed-off-by: Gautham R Shenoy Signed-off-by: Aneesh Kumar K.V --- hw/virtio-9p-local.c |7 ++ hw/virtio-9p.c | 54 - 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c index 0a9f111..43867be 100644 --- a/hw/virtio-9p-local.c +++ b/hw/virtio-9p-local.c @@ -245,6 +245,12 @@ static int local_utime(void *opaque, const char *path, return utime(rpath(path), buf); } +static int local_remove(void *opaque, const char *path) +{ +return remove(rpath(path)); +} + + static V9fsPosixFileOperations ops = { .lstat = local_lstat, .setuid = local_setuid, @@ -272,6 +278,7 @@ static V9fsPosixFileOperations ops = { .rename = local_rename, .chown = local_chown, .utime = local_utime, +.remove = local_remove, }; V9fsPosixFileOperations *virtio_9p_init_local(const char *path) diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c index 836d65b..50ee82a 100644 --- a/hw/virtio-9p.c +++ b/hw/virtio-9p.c @@ -237,6 +237,11 @@ static int posix_utime(V9fsState *s, V9fsString *path, return s->ops->utime(s->ops->opaque, path->data, buf); } +static int posix_remove(V9fsState *s, V9fsString *path) +{ +return s->ops->remove(s->ops->opaque, path->data); +} + static void v9fs_string_init(V9fsString *str) { str->data = NULL; @@ -1719,10 +1724,55 @@ static void v9fs_flush(V9fsState *s, V9fsPDU *pdu) pprint_pdu(pdu); } +typedef struct V9fsRemoveState { +V9fsPDU *pdu; +size_t offset; +int32_t fid; +V9fsFidState *fidp; +} V9fsRemoveState; + +static void v9fs_remove_post_remove(V9fsState *s, V9fsRemoveState *vs, +int err) +{ +if (err) { +err = -errno; +goto out; +} + +err = free_fid(s, vs->fid); +if (err < 0) +goto out; + +err = vs->offset; +out: +complete_pdu(s, vs->pdu, err); +qemu_free(vs); +} + static void v9fs_remove(V9fsState *s, V9fsPDU *pdu) { -if (debug_9p_pdu) - pprint_pdu(pdu); +V9fsRemoveState *vs; +int err = 0; + +vs = qemu_malloc(sizeof(*vs)); +vs->pdu = pdu; +vs->offset = 7; + +pdu_unmarshal(vs->pdu, vs->offset, "d", &vs->fid); + +vs->fidp = lookup_fid(s, vs->fid); +if (vs->fidp == NULL) { +err = -EINVAL; +goto out; +} + +err = posix_remove(s, &vs->fidp->path); +v9fs_remove_post_remove(s, vs, err); +return; + +out: +complete_pdu(s, pdu, err); +qemu_free(vs); } static mode_t v9mode_to_mode(uint32_t mode, V9fsString *extension) -- 1.6.5.2
[Qemu-devel] [PATCH 09/17] virtio-9p: Implement P9_TWRITE
This gets write to file to work Signed-off-by: Anthony Liguori Signed-off-by: Venkateswararao Jujjuri Signed-off-by: Aneesh Kumar K.V --- hw/virtio-9p-local.c |7 hw/virtio-9p.c | 97 - 2 files changed, 102 insertions(+), 2 deletions(-) diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c index 90664a0..441d22d 100644 --- a/hw/virtio-9p-local.c +++ b/hw/virtio-9p-local.c @@ -129,6 +129,12 @@ static off_t local_lseek(void *opaque, int fd, off_t offset, int whence) return lseek(fd, offset, whence); } +static ssize_t local_writev(void *opaque, int fd, const struct iovec *iov, + int iovcnt) +{ +return writev(fd, iov, iovcnt); +} + static V9fsPosixFileOperations ops = { .lstat = local_lstat, .setuid = local_setuid, @@ -143,6 +149,7 @@ static V9fsPosixFileOperations ops = { .seekdir = local_seekdir, .readv = local_readv, .lseek = local_lseek, +.writev = local_writev, }; V9fsPosixFileOperations *virtio_9p_init_local(const char *path) diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c index 7822a00..b0662ba 100644 --- a/hw/virtio-9p.c +++ b/hw/virtio-9p.c @@ -168,6 +168,12 @@ static off_t posix_lseek(V9fsState *s, int fd, off_t offset, int whence) return s->ops->lseek(s->ops->opaque, fd, offset, whence); } +static int posix_writev(V9fsState *s, int fd, const struct iovec *iov, + int iovcnt) +{ +return s->ops->writev(s->ops->opaque, fd, iov, iovcnt); +} + static void v9fs_string_init(V9fsString *str) { str->data = NULL; @@ -1321,10 +1327,97 @@ out: complete_pdu(s, pdu, err); } +typedef struct V9fsWriteState { +V9fsPDU *pdu; +size_t offset; +int32_t fid; +int32_t len; +int32_t count; +int32_t total; +int64_t off; +V9fsFidState *fidp; +struct iovec iov[128]; /* FIXME: bad, bad, bad */ +struct iovec *sg; +int cnt; +} V9fsWriteState; + +static void v9fs_write_post_writev(V9fsState *s, V9fsWriteState *vs, + ssize_t err) +{ +BUG_ON(vs->len < 0); +vs->total += vs->len; +vs->sg = adjust_sg(vs->sg, vs->len, &vs->cnt); +if (vs->total < vs->count && vs->len > 0) { +do { +if (0) +print_sg(vs->sg, vs->cnt); +vs->len = posix_writev(s, vs->fidp->fd, vs->sg, vs->cnt); +} while (vs->len == -1 && errno == EINTR); +v9fs_write_post_writev(s, vs, err); +} +vs->offset += pdu_marshal(vs->pdu, vs->offset, "d", vs->total); + +err = vs->offset; +complete_pdu(s, vs->pdu, err); +qemu_free(vs); +} + +static void v9fs_write_post_lseek(V9fsState *s, V9fsWriteState *vs, ssize_t err) +{ +BUG_ON(err == -1); + +vs->sg = cap_sg(vs->sg, vs->count, &vs->cnt); + +if (vs->total < vs->count) { +do { +if (0) +print_sg(vs->sg, vs->cnt); +vs->len = posix_writev(s, vs->fidp->fd, vs->sg, vs->cnt); +} while (vs->len == -1 && errno == EINTR); + +v9fs_write_post_writev(s, vs, err); +return; +} + +complete_pdu(s, vs->pdu, err); +qemu_free(vs); +} + static void v9fs_write(V9fsState *s, V9fsPDU *pdu) { -if (debug_9p_pdu) - pprint_pdu(pdu); +V9fsWriteState *vs; +ssize_t err; + +vs = qemu_malloc(sizeof(*vs)); + +vs->pdu = pdu; +vs->offset = 7; +vs->sg = vs->iov; +vs->total = 0; +vs->len = 0; + +pdu_unmarshal(vs->pdu, vs->offset, "dqdv", &vs->fid, &vs->off, &vs->count, +vs->sg, &vs->cnt); + +vs->fidp = lookup_fid(s, vs->fid); +if (vs->fidp == NULL) { +err = -EINVAL; +goto out; +} + +if (vs->fidp->fd == -1) { + err = -EINVAL; + goto out; +} + +err = posix_lseek(s, vs->fidp->fd, vs->off, SEEK_SET); + +v9fs_write_post_lseek(s, vs, err); +return; + +out: +complete_pdu(s, vs->pdu, err); +qemu_free(vs); } static void v9fs_create(V9fsState *s, V9fsPDU *pdu) -- 1.6.5.2
[Qemu-devel] [PATCH 13/17] virtio-9p: Implement P9_TFLUSH
Don't do anything special for flush Signed-off-by: Anthony Liguori Signed-off-by: Aneesh Kumar K.V --- hw/virtio-9p.c |5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c index 50ee82a..72e0339 100644 --- a/hw/virtio-9p.c +++ b/hw/virtio-9p.c @@ -1720,10 +1720,11 @@ out: static void v9fs_flush(V9fsState *s, V9fsPDU *pdu) { -if (debug_9p_pdu) - pprint_pdu(pdu); +/* A nop call with no return */ +complete_pdu(s, pdu, 7); } + typedef struct V9fsRemoveState { V9fsPDU *pdu; size_t offset; -- 1.6.5.2
[Qemu-devel] [PATCH 00/17][RFC] virtio-9p: paravirtual filesystem passthrough
This is a refresh of an older series
[Qemu-devel] [PATCH 11/17] virtio-9p: Implement P9_TWSTAT
This gets file and directory creation to work Signed-off-by: Anthony Liguori Signed-off-by: Gautham R Shenoy Signed-off-by: Aneesh Kumar K.V --- hw/virtio-9p-local.c | 42 + hw/virtio-9p.c | 236 +- 2 files changed, 274 insertions(+), 4 deletions(-) diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c index 606f5be..0a9f111 100644 --- a/hw/virtio-9p-local.c +++ b/hw/virtio-9p-local.c @@ -207,6 +207,44 @@ static int local_link(void *opaque, const char *oldpath, const char *newpath) return err; } +static int local_truncate(void *opaque, const char *path, off_t size) +{ +return truncate(rpath(path), size); +} + +static int local_rename(void *opaque, const char *oldpath, + const char *newpath) +{ +char *tmp; +int err; + +tmp = strdup(rpath(oldpath)); +if (tmp == NULL) + return -1; + +err = rename(tmp, rpath(newpath)); +if (err == -1) { + int serrno = errno; + free(tmp); + errno = serrno; +} else + free(tmp); + +return err; + +} + +static int local_chown(void *opaque, const char *path, uid_t uid, gid_t gid) +{ +return chown(rpath(path), uid, gid); +} + +static int local_utime(void *opaque, const char *path, + const struct utimbuf *buf) +{ +return utime(rpath(path), buf); +} + static V9fsPosixFileOperations ops = { .lstat = local_lstat, .setuid = local_setuid, @@ -230,6 +268,10 @@ static V9fsPosixFileOperations ops = { .open2 = local_open2, .symlink = local_symlink, .link = local_link, +.truncate = local_truncate, +.rename = local_rename, +.chown = local_chown, +.utime = local_utime, }; V9fsPosixFileOperations *virtio_9p_init_local(const char *path) diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c index 841fcde..836d65b 100644 --- a/hw/virtio-9p.c +++ b/hw/virtio-9p.c @@ -215,6 +215,28 @@ static int posix_link(V9fsState *s, V9fsString *oldpath, V9fsString *newpath) return s->ops->link(s->ops->opaque, oldpath->data, newpath->data); } +static int posix_truncate(V9fsState *s, V9fsString *path, off_t size) +{ +return s->ops->truncate(s->ops->opaque, path->data, size); +} + +static int posix_rename(V9fsState *s, V9fsString *oldpath, + V9fsString *newpath) +{ +return s->ops->rename(s->ops->opaque, oldpath->data, newpath->data); +} + +static int posix_chown(V9fsState *s, V9fsString *path, uid_t uid, gid_t gid) +{ +return s->ops->chown(s->ops->opaque, path->data, uid, gid); +} + +static int posix_utime(V9fsState *s, V9fsString *path, + const struct utimbuf *buf) +{ +return s->ops->utime(s->ops->opaque, path->data, buf); +} + static void v9fs_string_init(V9fsString *str) { str->data = NULL; @@ -398,7 +420,8 @@ static size_t pdu_unpack(void *dst, V9fsPDU *pdu, size_t offset, size_t size) } /* FIXME i can do this with less variables */ -static size_t pdu_pack(V9fsPDU *pdu, size_t offset, const void *src, size_t size) +static size_t pdu_pack(V9fsPDU *pdu, size_t offset, const void *src, + size_t size) { struct iovec *sg = pdu->elem.in_sg; size_t off = 0; @@ -1615,7 +1638,8 @@ static void v9fs_create_post_lstat(V9fsState *s, V9fsCreateState *vs, int err) uint32_t major, minor; mode_t nmode = 0; -if (sscanf(vs->extension.data, "%c %u %u", &ctype, &major, &minor) != 3) { +if (sscanf(vs->extension.data, "%c %u %u", &ctype, &major, + &minor) != 3) { err = -errno; v9fs_post_create(s, vs, err); } @@ -1701,10 +1725,214 @@ static void v9fs_remove(V9fsState *s, V9fsPDU *pdu) pprint_pdu(pdu); } +static mode_t v9mode_to_mode(uint32_t mode, V9fsString *extension) +{ + mode_t ret; + + ret = mode & 0777; + if (mode & P9_STAT_MODE_DIR) + ret |= S_IFDIR; + + if (dotu) { + if (mode & P9_STAT_MODE_SYMLINK) + ret |= S_IFLNK; + if (mode & P9_STAT_MODE_SOCKET) + ret |= S_IFSOCK; + if (mode & P9_STAT_MODE_NAMED_PIPE) + ret |= S_IFIFO; + if (mode & P9_STAT_MODE_DEVICE) { + if (extension && extension->data[0] == 'c') + ret |= S_IFCHR; + else + ret |= S_IFBLK; + } + } + + if (!(ret&~0777)) + ret |= S_IFREG; + + if (mode & P9_STAT_MODE_SETUID) + ret |= S_ISUID; + if (mode & P9_STAT_MODE_SETGID) + ret |= S_ISGID; + if (mode & P9_STAT_MODE_SETVTX) + ret |= S_ISVTX; + + return ret; +} + +typedef struct V9fsWstatState +{ +V9fsPDU *pdu; +size_t offset; +int32_t fi
[Qemu-devel] [PATCH 14/17] virtio-9p: Add multiple mount point support
From: Aneesh Kumar K.V This patch add a mount tag name in 9p config space. This tag should uniquely identify the mount point and should be used in the mount command as the device name Qemu command line for specifying 9p share directory now becomes -device virtio-9p-pci,share_path=/mnt/,mount_tag=v_mnt -device virtio-9p-pci,share_path=/tmp/,mount_tag=v_tmp NOTE: We now limit tag name to 32 characters because of virtio config space limitation. Signed-off-by: Aneesh Kumar K.V --- hw/9p.h | 26 +++ hw/virtio-9p-local.c | 97 hw/virtio-9p.c | 204 - hw/virtio-9p.h | 141 --- hw/virtio-pci.c |8 +- hw/virtio.h |3 +- 6 files changed, 293 insertions(+), 186 deletions(-) create mode 100644 hw/9p.h diff --git a/hw/9p.h b/hw/9p.h new file mode 100644 index 000..f0ff45b --- /dev/null +++ b/hw/9p.h @@ -0,0 +1,26 @@ +/* + * Virtio 9p + * + * Copyright IBM, Corp. 2010 + * + * Authors: + * Aneesh Kumar K.V + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + * + */ + +#ifndef QEMU_9P_H +#define QEMU_9P_H + +#include + +typedef struct V9fsConf +{ +char *share_path; +/* tag name for the device */ +char *tag; +} V9fsConf; + +#endif diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c index 43867be..d89a735 100644 --- a/hw/virtio-9p-local.c +++ b/hw/virtio-9p-local.c @@ -22,22 +22,20 @@ #include #include -static const char *base_path; - -static const char *rpath(const char *path) +static const char *rpath(V9fsState *s, const char *path) { /* FIXME: so wrong... */ static char buffer[4096]; -snprintf(buffer, sizeof(buffer), "%s/%s", base_path, path); +snprintf(buffer, sizeof(buffer), "%s/%s", s->fs_root, path); return buffer; } -static int local_lstat(void *opaque, const char *path, struct stat *stbuf) +static int local_lstat(V9fsState *s, const char *path, struct stat *stbuf) { -return lstat(rpath(path), stbuf); +return lstat(rpath(s, path), stbuf); } -static int local_setuid(void *opaque, uid_t uid) +static int local_setuid(V9fsState *s, uid_t uid) { struct passwd *pw; gid_t groups[33]; @@ -72,86 +70,86 @@ static int local_setuid(void *opaque, uid_t uid) return 0; } -static ssize_t local_readlink(void *opaque, const char *path, +static ssize_t local_readlink(V9fsState *s, const char *path, char *buf, size_t bufsz) { -return readlink(rpath(path), buf, bufsz); +return readlink(rpath(s, path), buf, bufsz); } -static int local_close(void *opaque, int fd) +static int local_close(V9fsState *s, int fd) { return close(fd); } -static int local_closedir(void *opaque, DIR *dir) +static int local_closedir(V9fsState *s, DIR *dir) { return closedir(dir); } -static int local_open(void *opaque, const char *path, int flags) +static int local_open(V9fsState *s, const char *path, int flags) { -return open(rpath(path), flags); +return open(rpath(s, path), flags); } -static DIR *local_opendir(void *opaque, const char *path) +static DIR *local_opendir(V9fsState *s, const char *path) { -return opendir(rpath(path)); +return opendir(rpath(s, path)); } -static void local_rewinddir(void *opaque, DIR *dir) +static void local_rewinddir(V9fsState *s, DIR *dir) { return rewinddir(dir); } -static off_t local_telldir(void *opaque, DIR *dir) +static off_t local_telldir(V9fsState *s, DIR *dir) { return telldir(dir); } -static struct dirent *local_readdir(void *opaque, DIR *dir) +static struct dirent *local_readdir(V9fsState *s, DIR *dir) { return readdir(dir); } -static void local_seekdir(void *opaque, DIR *dir, off_t off) +static void local_seekdir(V9fsState *s, DIR *dir, off_t off) { return seekdir(dir, off); } -static ssize_t local_readv(void *opaque, int fd, const struct iovec *iov, +static ssize_t local_readv(V9fsState *s, int fd, const struct iovec *iov, int iovcnt) { return readv(fd, iov, iovcnt); } -static off_t local_lseek(void *opaque, int fd, off_t offset, int whence) +static off_t local_lseek(V9fsState *s, int fd, off_t offset, int whence) { return lseek(fd, offset, whence); } -static ssize_t local_writev(void *opaque, int fd, const struct iovec *iov, +static ssize_t local_writev(V9fsState *s, int fd, const struct iovec *iov, int iovcnt) { return writev(fd, iov, iovcnt); } -static int local_chmod(void *opaque, const char *path, mode_t mode) +static int local_chmod(V9fsState *s, const char *path, mode_t mode) { -return chmod(rpath(path), mode); +return chmod(rpath(s, path), mode); } -static int local_mknod(void *opaque, const char *path, mode_t mode, dev_t dev) +static int local_mknod(V9fsState *s, const char *
[Qemu-devel] [PATCH 17/17] Implement sync support in 9p server
From: M. Mohan Kumar When wstat is called with stat field values set to 'don't touch' pattern, 9p Server interprets it as a request to guarantee that the contents of the associated file are committed to stable storage before the Rwstat message is returned. Implement this feature in the server side. Signed-off-by: M. Mohan Kumar Signed-off-by: Aneesh Kumar K.V --- hw/virtio-9p-local.c |6 ++ hw/virtio-9p.c | 34 ++ hw/virtio-9p.h |1 + 3 files changed, 41 insertions(+), 0 deletions(-) diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c index d89a735..4317ea3 100644 --- a/hw/virtio-9p-local.c +++ b/hw/virtio-9p-local.c @@ -249,6 +249,11 @@ static int local_remove(V9fsState *s, const char *path) } +static int local_fsync(V9fsState *s, int fd) +{ +return fsync(fd); +} + static V9fsPosixFileOperations ops = { .lstat = local_lstat, .setuid = local_setuid, @@ -277,6 +282,7 @@ static V9fsPosixFileOperations ops = { .chown = local_chown, .utime = local_utime, .remove = local_remove, +.fsync = local_fsync, }; V9fsPosixFileOperations *virtio_9p_init_local(const char *path) diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c index 8a96645..94c9c58 100644 --- a/hw/virtio-9p.c +++ b/hw/virtio-9p.c @@ -172,6 +172,11 @@ static int posix_remove(V9fsState *s, V9fsString *path) return s->ops->remove(s, path->data); } +static int posix_fsync(V9fsState *s, int fd) +{ +return s->ops->fsync(s, fd); +} + static void v9fs_string_init(V9fsString *str) { str->data = NULL; @@ -1892,6 +1897,29 @@ out: qemu_free(vs); } +static int donttouch_stat(V9fsStat *stat) +{ +if (stat->type == -1 && + stat->dev == -1 && + stat->qid.type == -1 && + stat->qid.version == -1 && + stat->qid.path == -1 && + stat->mode == -1 && + stat->atime == -1 && + stat->mtime == -1 && + stat->length == -1 && + !stat->name.size && + !stat->uid.size && + !stat->gid.size && + !stat->muid.size && + stat->n_uid == -1 && + stat->n_gid == -1 && + stat->n_muid == -1) + return 1; + else + return 0; +} + static void v9fs_wstat(V9fsState *s, V9fsPDU *pdu) { V9fsWstatState *vs; @@ -1909,6 +1937,12 @@ static void v9fs_wstat(V9fsState *s, V9fsPDU *pdu) goto out; } +/* do we need to sync the file? */ +if (donttouch_stat(&vs->v9stat)) { +posix_fsync(s, vs->fidp->fd); +goto out; +} + if (vs->v9stat.mode != -1) { if (vs->v9stat.mode & P9_STAT_MODE_DIR && vs->fidp->dir == NULL) { err = -EIO; diff --git a/hw/virtio-9p.h b/hw/virtio-9p.h index 42ca887..113b0d5 100644 --- a/hw/virtio-9p.h +++ b/hw/virtio-9p.h @@ -182,6 +182,7 @@ typedef struct V9fsPosixFileOpertions int (*fstat)(V9fsState *, int, struct stat *); int (*rename)(V9fsState *, const char *, const char *); int (*truncate)(V9fsState *, const char *, off_t); +int (*fsync)(V9fsState *, int); void *opaque; } V9fsPosixFileOperations; -- 1.6.5.2
[Qemu-devel] [PATCH 07/17] virtio-9p: Implement P9_TREAD
Signed-off-by: Anthony Liguori Signed-off-by: Venkateswararao Jujjuri Signed-off-by: Aneesh Kumar K.V --- hw/virtio-9p-local.c | 37 hw/virtio-9p.c | 253 +- 2 files changed, 287 insertions(+), 3 deletions(-) diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c index 803b0ea..90664a0 100644 --- a/hw/virtio-9p-local.c +++ b/hw/virtio-9p-local.c @@ -98,6 +98,37 @@ static DIR *local_opendir(void *opaque, const char *path) return opendir(rpath(path)); } +static void local_rewinddir(void *opaque, DIR *dir) +{ +return rewinddir(dir); +} + +static off_t local_telldir(void *opaque, DIR *dir) +{ +return telldir(dir); +} + +static struct dirent *local_readdir(void *opaque, DIR *dir) +{ +return readdir(dir); +} + +static void local_seekdir(void *opaque, DIR *dir, off_t off) +{ +return seekdir(dir, off); +} + +static ssize_t local_readv(void *opaque, int fd, const struct iovec *iov, + int iovcnt) +{ +return readv(fd, iov, iovcnt); +} + +static off_t local_lseek(void *opaque, int fd, off_t offset, int whence) +{ +return lseek(fd, offset, whence); +} + static V9fsPosixFileOperations ops = { .lstat = local_lstat, .setuid = local_setuid, @@ -106,6 +137,12 @@ static V9fsPosixFileOperations ops = { .closedir = local_closedir, .open = local_open, .opendir = local_opendir, +.rewinddir = local_rewinddir, +.telldir = local_telldir, +.readdir = local_readdir, +.seekdir = local_seekdir, +.readv = local_readv, +.lseek = local_lseek, }; V9fsPosixFileOperations *virtio_9p_init_local(const char *path) diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c index 7709e4c..b30933a 100644 --- a/hw/virtio-9p.c +++ b/hw/virtio-9p.c @@ -137,6 +137,37 @@ static DIR *posix_opendir(V9fsState *s, V9fsString *path) return s->ops->opendir(s->ops->opaque, path->data); } +static void posix_rewinddir(V9fsState *s, DIR *dir) +{ +return s->ops->rewinddir(s->ops->opaque, dir); +} + +static off_t posix_telldir(V9fsState *s, DIR *dir) +{ +return s->ops->telldir(s->ops->opaque, dir); +} + +static struct dirent *posix_readdir(V9fsState *s, DIR *dir) +{ +return s->ops->readdir(s->ops->opaque, dir); +} + +static void posix_seekdir(V9fsState *s, DIR *dir, off_t off) +{ +return s->ops->seekdir(s->ops->opaque, dir, off); +} + +static int posix_readv(V9fsState *s, int fd, const struct iovec *iov, + int iovcnt) +{ +return s->ops->readv(s->ops->opaque, fd, iov, iovcnt); +} + +static off_t posix_lseek(V9fsState *s, int fd, off_t offset, int whence) +{ +return s->ops->lseek(s->ops->opaque, fd, offset, whence); +} + static void v9fs_string_init(V9fsString *str) { str->data = NULL; @@ -1050,14 +1081,230 @@ out: qemu_free(vs); } -static void v9fs_clunk(V9fsState *s, V9fsPDU *pdu) +static struct iovec *adjust_sg(struct iovec *sg, int len, int *iovcnt) { -if (debug_9p_pdu) - pprint_pdu(pdu); +while (len && *iovcnt) { + if (len < sg->iov_len) { + sg->iov_len -= len; + sg->iov_base += len; + len = 0; + } else { + len -= sg->iov_len; + sg++; + *iovcnt -= 1; + } +} + +return sg; +} + +static struct iovec *cap_sg(struct iovec *sg, int cap, int *cnt) +{ +int i; +int total = 0; + +for (i = 0; i < *cnt; i++) { + if ((total + sg[i].iov_len) > cap) { + sg[i].iov_len -= ((total + sg[i].iov_len) - cap); + i++; + break; + } + total += sg[i].iov_len; +} + +*cnt = i; + +return sg; +} + +static void print_sg(struct iovec *sg, int cnt) +{ +int i; + +printf("sg[%d]: {", cnt); +for (i = 0; i < cnt; i++) { + if (i) + printf(", "); + printf("(%p, %zd)", sg[i].iov_base, sg[i].iov_len); +} +printf("}\n"); +} + +typedef struct V9fsReadState { +V9fsPDU *pdu; +size_t offset; +int32_t fid; +int32_t count; +int32_t total; +int64_t off; +V9fsFidState *fidp; +struct iovec iov[128]; /* FIXME: bad, bad, bad */ +struct iovec *sg; +off_t dir_pos; +struct dirent *dent; +struct stat stbuf; +V9fsString name; +V9fsStat v9stat; +int32_t len; +int32_t cnt; +int32_t max_count; +} V9fsReadState; + +static void v9fs_read_post_readdir(V9fsState *, V9fsReadState *, ssize_t ); + +static void v9fs_read_post_seekdir(V9fsState *s, V9fsReadState *vs, ssize_t err) +{ +v9fs_stat_free(&vs->v9stat); +v9fs_string_free(&vs->name); +vs->offset += pdu_marshal(vs->pdu, vs->offset, "d", vs->count); +vs->offset += vs->count; +err = vs->offset; +complete_pdu(s, vs->pdu, err); +qemu_free(vs); +return; +} + +static void v9fs_read_post_dir_lstat(V9fsState *s, V9fsReadState *vs, + ssize_t err) +{ +BUG_ON(err == -1); +stat_to_v9stat(s, &vs->nam
[Qemu-devel] [PATCH 16/17] virtio-9p: Add support for hardlink
From: Aneesh Kumar K.V link count for files and directories are encoded as a tagged string in the extension field for a .u protocol extension. Signed-off-by: Aneesh Kumar K.V --- hw/virtio-9p.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c index 65f5827..8a96645 100644 --- a/hw/virtio-9p.c +++ b/hw/virtio-9p.c @@ -676,6 +676,9 @@ static void stat_to_v9stat(V9fsState *s, V9fsString *name, v9fs_string_sprintf(&v9stat->extension, "%c %u %u", S_ISCHR(stbuf->st_mode) ? 'c' : 'b', major(stbuf->st_rdev), minor(stbuf->st_rdev)); + } else if (S_ISDIR(stbuf->st_mode) || S_ISREG(stbuf->st_mode)) { + v9fs_string_sprintf(&v9stat->extension, "%s %u", + "HARDLINKCOUNT", stbuf->st_nlink); } } -- 1.6.5.2
[Qemu-devel] [PATCH 03/17] virtio-9p: Implement P9_TATTACH
Signed-off-by: Anthony Liguori Signed-off-by: Aneesh Kumar K.V --- Makefile.target |2 +- hw/virtio-9p-local.c | 84 +++ hw/virtio-9p.c | 155 +++--- hw/virtio-9p.h | 33 +++ 4 files changed, 264 insertions(+), 10 deletions(-) create mode 100644 hw/virtio-9p-local.c diff --git a/Makefile.target b/Makefile.target index 8f240ea..ff5fc4b 100644 --- a/Makefile.target +++ b/Makefile.target @@ -173,7 +173,7 @@ obj-y = vl.o async.o monitor.o pci.o pci_host.o pcie_host.o machine.o gdbstub.o # virtio has to be here due to weird dependency between PCI and virtio-net. # need to fix this properly obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-pci.o virtio-serial-bus.o -obj-y += virtio-9p.o virtio-9p-debug.o +obj-y += virtio-9p.o virtio-9p-debug.o virtio-9p-local.o obj-y += rwhandler.o obj-$(CONFIG_KVM) += kvm.o kvm-all.o obj-$(CONFIG_ISA_MMIO) += isa_mmio.o diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c new file mode 100644 index 000..204437c --- /dev/null +++ b/hw/virtio-9p-local.c @@ -0,0 +1,84 @@ +/* + * Virtio 9p Posix callback + * + * Copyright IBM, Corp. 2010 + * + * Authors: + * Anthony Liguori + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + * + */ +#include "virtio.h" +#include "pc.h" +#include "qemu_socket.h" +#include "virtio-9p.h" +#include +#include +#include +#include +#include +#include +#include + +static const char *base_path; + +static const char *rpath(const char *path) +{ +/* FIXME: so wrong... */ +static char buffer[4096]; +snprintf(buffer, sizeof(buffer), "%s/%s", base_path, path); +return buffer; +} + +static int local_lstat(void *opaque, const char *path, struct stat *stbuf) +{ +return lstat(rpath(path), stbuf); +} + +static int local_setuid(void *opaque, uid_t uid) +{ +struct passwd *pw; +gid_t groups[33]; +int ngroups; +static uid_t cur_uid = -1; + +if (cur_uid == uid) + return 0; + +if (setreuid(0, 0)) + return -1; + +pw = getpwuid(uid); +if (pw == NULL) + return -1; + +ngroups = 33; +if (getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups) == -1) + return -1; + +if (setgroups(ngroups, groups)) + return -1; + +if (setregid(-1, pw->pw_gid)) + return -1; + +if (setreuid(-1, uid)) + return -1; + +cur_uid = uid; + +return 0; +} + +static V9fsPosixFileOperations ops = { +.lstat = local_lstat, +.setuid = local_setuid, +}; + +V9fsPosixFileOperations *virtio_9p_init_local(const char *path) +{ + base_path = path; + return &ops; +} diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c index a057fbb..c63ac80 100644 --- a/hw/virtio-9p.c +++ b/hw/virtio-9p.c @@ -82,6 +82,7 @@ typedef struct V9fsState V9fsPDU pdus[MAX_REQ]; V9fsPDU *free_pdu; V9fsFidState *fid_list; +V9fsPosixFileOperations *ops; char *root; uid_t uid; } V9fsState; @@ -91,6 +92,123 @@ int debug_9p_pdu = 1; extern void pprint_pdu(V9fsPDU *pdu); +static int posix_lstat(V9fsState *s, V9fsString *path, struct stat *stbuf) +{ +return s->ops->lstat(s->ops->opaque, path->data, stbuf); +} + +static int posix_setuid(V9fsState *s, uid_t uid) +{ +return s->ops->setuid(s->ops->opaque, uid); +} + +static void v9fs_string_free(V9fsString *str) +{ +free(str->data); +str->data = NULL; +str->size = 0; +} + +static void v9fs_string_sprintf(V9fsString *str, const char *fmt, ...) +{ +va_list ap; +int err; + +v9fs_string_free(str); + +va_start(ap, fmt); +err = vasprintf(&str->data, fmt, ap); +BUG_ON(err == -1); +va_end(ap); + +str->size = err; +} + +static V9fsFidState *lookup_fid(V9fsState *s, int32_t fid) +{ +V9fsFidState *f; + +for (f = s->fid_list; f; f = f->next) { + if (f->fid == fid) { + posix_setuid(s, f->uid); + return f; + } +} + +return NULL; +} + +static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid) +{ +V9fsFidState *f; + +f = lookup_fid(s, fid); +if (f) + return NULL; + +f = qemu_mallocz(sizeof(V9fsFidState)); +BUG_ON(f == NULL); + +f->fid = fid; +f->fd = -1; +f->dir = NULL; + +f->next = s->fid_list; +s->fid_list = f; + +return f; +} + +#define P9_QID_TYPE_DIR0x80 +#define P9_QID_TYPE_SYMLINK0x02 + +#define P9_STAT_MODE_DIR 0x8000 +#define P9_STAT_MODE_APPEND0x4000 +#define P9_STAT_MODE_EXCL 0x2000 +#define P9_STAT_MODE_MOUNT 0x1000 +#define P9_STAT_MODE_AUTH 0x0800 +#define P9_STAT_MODE_TMP 0x0400 +#define P9_STAT_MODE_SYMLINK 0x0200 +#define P9_STAT_MODE_LINK 0x0100 +#define P9_STAT_MODE_DEVICE0x0080 +#define P9_STAT_MODE_NAMED_PIPE0x0020 +#define P9_STAT_MODE_SOCKET0x0010 +#define
[Qemu-devel] [PATCH 00/17][RFC] virtio-9p: paravirtual filesystem passthrough
This patch series adds a paravirtual file system passthrough mechanism to QEMU based on the 9P protocol. This an RFC series with a few known issues. Right now, all I/O is implemented in the VCPU thread. We've modified the protocol handlers so that we can support dispatch I/O in a thread pool but we wanted to send out the series for initial review before completing that work. We also do not have a very good mechanism for specifying the share path. Suggestions would be welcome here. This patch set should work with any recent Linux kernel as virtio-9p has been supported for a few kernel releases now.
[Qemu-devel] [PATCH 05/17] virtio-9p: Implement P9_TWALK
Signed-off-by: Anthony Liguori Signed-off-by: Gautham R Shenoy Signed-off-by: Aneesh Kumar K.V --- hw/virtio-9p-local.c | 12 +++ hw/virtio-9p.c | 219 +- 2 files changed, 229 insertions(+), 2 deletions(-) diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c index 9752f76..ac0dce5 100644 --- a/hw/virtio-9p-local.c +++ b/hw/virtio-9p-local.c @@ -78,10 +78,22 @@ static ssize_t local_readlink(void *opaque, const char *path, return readlink(rpath(path), buf, bufsz); } +static int local_close(void *opaque, int fd) +{ +return close(fd); +} + +static int local_closedir(void *opaque, DIR *dir) +{ +return closedir(dir); +} + static V9fsPosixFileOperations ops = { .lstat = local_lstat, .setuid = local_setuid, .readlink = local_readlink, +.close = local_close, +.closedir = local_closedir, }; V9fsPosixFileOperations *virtio_9p_init_local(const char *path) diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c index 10bcd89..685646a 100644 --- a/hw/virtio-9p.c +++ b/hw/virtio-9p.c @@ -117,6 +117,22 @@ static ssize_t posix_readlink(V9fsState *s, V9fsString *path, V9fsString *buf) return len; } +static int posix_close(V9fsState *s, int fd) +{ +return s->ops->close(s->ops->opaque, fd); +} + +static int posix_closedir(V9fsState *s, DIR *dir) +{ +return s->ops->closedir(s->ops->opaque, dir); +} + +static void v9fs_string_init(V9fsString *str) +{ +str->data = NULL; +str->size = 0; +} + static void v9fs_string_free(V9fsString *str) { free(str->data); @@ -144,6 +160,12 @@ static void v9fs_string_sprintf(V9fsString *str, const char *fmt, ...) str->size = err; } +static void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs) +{ +v9fs_string_free(lhs); +v9fs_string_sprintf(lhs, "%s", rhs->data); +} + static size_t v9fs_string_size(V9fsString *str) { return str->size; @@ -184,6 +206,31 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid) return f; } +static int free_fid(V9fsState *s, int32_t fid) +{ +V9fsFidState **fidpp, *fidp; + +for (fidpp = &s->fid_list; *fidpp; fidpp = &(*fidpp)->next) { + if ((*fidpp)->fid == fid) + break; +} + +if (*fidpp == NULL) + return -ENOENT; + +fidp = *fidpp; +*fidpp = fidp->next; + +if (fidp->fd != -1) + posix_close(s, fidp->fd); +if (fidp->dir) + posix_closedir(s, fidp->dir); +v9fs_string_free(&fidp->path); +qemu_free(fidp); + +return 0; +} + #define P9_QID_TYPE_DIR0x80 #define P9_QID_TYPE_SYMLINK0x02 @@ -691,10 +738,178 @@ out: qemu_free(vs); } +typedef struct V9fsWalkState { +V9fsPDU *pdu; +size_t offset; +int32_t fid; +int32_t newfid; +int16_t nwnames; +int name_idx; +V9fsQID *qids; +V9fsFidState *fidp; +V9fsFidState *newfidp; +V9fsString path; +V9fsString *wnames; +struct stat stbuf; +} V9fsWalkState; + +static void v9fs_walk_complete(V9fsState *s, V9fsWalkState *vs, int err) +{ +complete_pdu(s, vs->pdu, err); + +if(vs->nwnames) { +for (vs->name_idx = 0; vs->name_idx < vs->nwnames; vs->name_idx++) +v9fs_string_free(&vs->wnames[vs->name_idx]); + +qemu_free(vs->wnames); +qemu_free(vs->qids); +} +} + +static void v9fs_walk_marshal(V9fsWalkState *vs) +{ +int i; +vs->offset = 7; +vs->offset += pdu_marshal(vs->pdu, vs->offset, "w", vs->nwnames); + +for (i = 0; i < vs->nwnames; i++) +vs->offset += pdu_marshal(vs->pdu, vs->offset, "Q", &vs->qids[i]); +} + +static void v9fs_walk_post_newfid_lstat(V9fsState *s, V9fsWalkState *vs, +int err) +{ +if (err == -1) { +free_fid(s, vs->newfid); +v9fs_string_free(&vs->path); +err = -ENOENT; +goto out; +} + +stat_to_qid(&vs->stbuf, &vs->qids[vs->name_idx]); + +vs->name_idx++; +if (vs->name_idx < vs->nwnames) { +v9fs_string_sprintf(&vs->path, "%s/%s", vs->newfidp->path.data, +vs->wnames[vs->name_idx].data); +v9fs_string_copy(&vs->newfidp->path, &vs->path); + +err = posix_lstat(s, &vs->newfidp->path, &vs->stbuf); +v9fs_walk_post_newfid_lstat(s, vs, err); +return; +} + +v9fs_string_free(&vs->path); +v9fs_walk_marshal(vs); +err = vs->offset; +out: +v9fs_walk_complete(s, vs, err); +} + +static void v9fs_walk_post_oldfid_lstat(V9fsState *s, V9fsWalkState *vs, +int err) +{ +if (err == -1) { +v9fs_string_free(&vs->path); +err = -ENOENT; +goto out; +} + +stat_to_qid(&vs->stbuf, &vs->qids[vs->name_idx]); +vs->name_idx++; +if (vs->name_idx < vs->nwnames) { + +v9fs_string_sprintf(&vs->path, "%s/%s", +vs->fidp->path.data, vs->wnames[
[Qemu-devel] [PATCH 06/17] virtio-9p: Implement P9_TOPEN
Signed-off-by: Anthony Liguori Signed-off-by: Gautham R Shenoy Signed-off-by: Aneesh Kumar K.V --- hw/virtio-9p-local.c | 12 hw/virtio-9p.c | 141 -- 2 files changed, 149 insertions(+), 4 deletions(-) diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c index ac0dce5..803b0ea 100644 --- a/hw/virtio-9p-local.c +++ b/hw/virtio-9p-local.c @@ -88,12 +88,24 @@ static int local_closedir(void *opaque, DIR *dir) return closedir(dir); } +static int local_open(void *opaque, const char *path, int flags) +{ +return open(rpath(path), flags); +} + +static DIR *local_opendir(void *opaque, const char *path) +{ +return opendir(rpath(path)); +} + static V9fsPosixFileOperations ops = { .lstat = local_lstat, .setuid = local_setuid, .readlink = local_readlink, .close = local_close, .closedir = local_closedir, +.open = local_open, +.opendir = local_opendir, }; V9fsPosixFileOperations *virtio_9p_init_local(const char *path) diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c index 685646a..7709e4c 100644 --- a/hw/virtio-9p.c +++ b/hw/virtio-9p.c @@ -127,6 +127,16 @@ static int posix_closedir(V9fsState *s, DIR *dir) return s->ops->closedir(s->ops->opaque, dir); } +static int posix_open(V9fsState *s, V9fsString *path, int flags) +{ +return s->ops->open(s->ops->opaque, path->data, flags); +} + +static DIR *posix_opendir(V9fsState *s, V9fsString *path) +{ +return s->ops->opendir(s->ops->opaque, path->data); +} + static void v9fs_string_init(V9fsString *str) { str->data = NULL; @@ -912,14 +922,137 @@ out: v9fs_walk_complete(s, vs, err); } -static void v9fs_clunk(V9fsState *s, V9fsPDU *pdu) +typedef struct V9fsOpenState { + V9fsPDU *pdu; + size_t offset; + int32_t fid; + int8_t mode; + V9fsFidState *fidp; + V9fsQID qid; + struct stat stbuf; + +} V9fsOpenState; + +enum { + Oread = 0x00, + Owrite = 0x01, + Ordwr = 0x02, + Oexec = 0x03, + Oexcl = 0x04, + Otrunc = 0x10, + Orexec = 0x20, + Orclose = 0x40, + Oappend = 0x80, +}; + +static int omode_to_uflags(int8_t mode) { -if (debug_9p_pdu) - pprint_pdu(pdu); +int ret = 0; + +switch (mode & 3) { +case Oread: + ret = O_RDONLY; + break; +case Ordwr: + ret = O_RDWR; + break; +case Owrite: + ret = O_WRONLY; + break; +case Oexec: + ret = O_RDONLY; + break; +} + +if (mode & Otrunc) + ret |= O_TRUNC; + +if (mode & Oappend) + ret |= O_APPEND; + +if (mode & Oexcl) + ret |= O_EXCL; + +return ret; +} + +static void v9fs_open_post_opendir(V9fsState *s, V9fsOpenState *vs, int err) +{ +if (vs->fidp->dir == NULL) { +err = -errno; +goto out; +} + +vs->offset += pdu_marshal(vs->pdu, vs->offset, "Qd", &vs->qid, 0); +err = vs->offset; +out: +complete_pdu(s, vs->pdu, err); +qemu_free(vs); + +} + +static void v9fs_open_post_open(V9fsState *s, V9fsOpenState *vs, int err) +{ +if (vs->fidp->fd == -1) { +err = -errno; +goto out; +} + +vs->offset += pdu_marshal(vs->pdu, vs->offset, "Qd", &vs->qid, 0); +err = vs->offset; +out: +complete_pdu(s, vs->pdu, err); +qemu_free(vs); +} + +static void v9fs_open_post_lstat(V9fsState *s, V9fsOpenState *vs, int err) +{ +BUG_ON(err == -1); + +stat_to_qid(&vs->stbuf, &vs->qid); + +if (S_ISDIR(vs->stbuf.st_mode)) { +vs->fidp->dir = posix_opendir(s, &vs->fidp->path); +v9fs_open_post_opendir(s, vs, err); +} else { +vs->fidp->fd = posix_open(s, &vs->fidp->path, +omode_to_uflags(vs->mode)); +v9fs_open_post_open(s, vs, err); +} + } static void v9fs_open(V9fsState *s, V9fsPDU *pdu) -{if (debug_9p_pdu) +{ + +V9fsOpenState *vs; +ssize_t err = 0; + + +vs = qemu_malloc(sizeof(*vs)); +vs->pdu = pdu; +vs->offset = 7; + +pdu_unmarshal(vs->pdu, vs->offset, "db", &vs->fid, &vs->mode); + +vs->fidp = lookup_fid(s, vs->fid); +if (vs->fidp == NULL) { +err = -ENOENT; +goto out; +} + +err = posix_lstat(s, &vs->fidp->path, &vs->stbuf); + +v9fs_open_post_lstat(s, vs, err); +return; +out: +complete_pdu(s, pdu, err); +qemu_free(vs); +} + +static void v9fs_clunk(V9fsState *s, V9fsPDU *pdu) +{ +if (debug_9p_pdu) pprint_pdu(pdu); } -- 1.6.5.2
[Qemu-devel] [PATCH 01/17] vitio-9p: Add a virtio 9p device to qemu
This patch doesn't implement the 9p protocol handling code. It add a simple device which dump the protocl data Signed-off-by: Anthony Liguori Signed-off-by: Aneesh Kumar K.V --- Makefile.target |1 + hw/virtio-9p-debug.c | 442 ++ hw/virtio-9p.c | 275 +++ hw/virtio-9p.h | 70 hw/virtio-pci.c | 25 +++ hw/virtio.h |1 + 6 files changed, 814 insertions(+), 0 deletions(-) create mode 100644 hw/virtio-9p-debug.c create mode 100644 hw/virtio-9p.c create mode 100644 hw/virtio-9p.h diff --git a/Makefile.target b/Makefile.target index 4c4d397..8f240ea 100644 --- a/Makefile.target +++ b/Makefile.target @@ -173,6 +173,7 @@ obj-y = vl.o async.o monitor.o pci.o pci_host.o pcie_host.o machine.o gdbstub.o # virtio has to be here due to weird dependency between PCI and virtio-net. # need to fix this properly obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-pci.o virtio-serial-bus.o +obj-y += virtio-9p.o virtio-9p-debug.o obj-y += rwhandler.o obj-$(CONFIG_KVM) += kvm.o kvm-all.o obj-$(CONFIG_ISA_MMIO) += isa_mmio.o diff --git a/hw/virtio-9p-debug.c b/hw/virtio-9p-debug.c new file mode 100644 index 000..060337f --- /dev/null +++ b/hw/virtio-9p-debug.c @@ -0,0 +1,442 @@ +/* + * Virtio 9p PDU debug + * + * Copyright IBM, Corp. 2010 + * + * Authors: + * Anthony Liguori + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + * + */ +#include "virtio.h" +#include "pc.h" +#include "virtio-9p.h" + +#include +#include + +#define BUG_ON(cond) assert(!(cond)) + +extern int dotu; +static FILE *llogfile; + +static struct iovec *get_sg(V9fsPDU *pdu, int rx) +{ +if (rx) + return pdu->elem.in_sg; +return pdu->elem.out_sg; +} + +static void pprint_int8(V9fsPDU *pdu, int rx, size_t *offsetp, + const char *name) +{ +struct iovec *sg = get_sg(pdu, rx); +size_t offset = *offsetp; +int8_t value; + +BUG_ON((offset + sizeof(value)) > sg[0].iov_len); + +memcpy(&value, sg[0].iov_base + offset, sizeof(value)); +offset += sizeof(value); + +fprintf(llogfile, "%s=0x%x", name, value); + +*offsetp = offset; +} + +static void pprint_int16(V9fsPDU *pdu, int rx, size_t *offsetp, + const char *name) +{ +struct iovec *sg = get_sg(pdu, rx); +size_t offset = *offsetp; +int16_t value; + +BUG_ON((offset + sizeof(value)) > sg[0].iov_len); + +memcpy(&value, sg[0].iov_base + offset, sizeof(value)); +offset += sizeof(value); + +fprintf(llogfile, "%s=0x%x", name, value); + +*offsetp = offset; +} + +static void pprint_int32(V9fsPDU *pdu, int rx, size_t *offsetp, + const char *name) +{ +struct iovec *sg = get_sg(pdu, rx); +size_t offset = *offsetp; +int32_t value; + +BUG_ON((offset + sizeof(value)) > sg[0].iov_len); + +memcpy(&value, sg[0].iov_base + offset, sizeof(value)); +offset += sizeof(value); + +fprintf(llogfile, "%s=0x%x", name, value); + +*offsetp = offset; +} + +static void pprint_int64(V9fsPDU *pdu, int rx, size_t *offsetp, + const char *name) +{ +struct iovec *sg = get_sg(pdu, rx); +size_t offset = *offsetp; +int64_t value; + +BUG_ON((offset + sizeof(value)) > sg[0].iov_len); + +memcpy(&value, sg[0].iov_base + offset, sizeof(value)); +offset += sizeof(value); + +fprintf(llogfile, "%s=0x%" PRIx64, name, value); + +*offsetp = offset; +} + +static void pprint_str(V9fsPDU *pdu, int rx, size_t *offsetp, const char *name) +{ +struct iovec *sg = get_sg(pdu, rx); +size_t offset = *offsetp; +int16_t size; +size_t result; + +BUG_ON((offset + 2) > sg[0].iov_len); +memcpy(&size, sg[0].iov_base + offset, 2); +offset += 2; + +BUG_ON((offset + size) > sg[0].iov_len); +fprintf(llogfile, "%s=", name); +result = fwrite(sg[0].iov_base + offset, 1, size, llogfile); +BUG_ON(result != size); +offset += size; + +*offsetp = offset; +} + +static void pprint_qid(V9fsPDU *pdu, int rx, size_t *offsetp, const char *name) +{ +fprintf(llogfile, "%s={", name); +pprint_int8(pdu, rx, offsetp, "type"); +pprint_int32(pdu, rx, offsetp, ", version"); +pprint_int64(pdu, rx, offsetp, ", path"); +fprintf(llogfile, "}"); +} + +static void pprint_stat(V9fsPDU *pdu, int rx, size_t *offsetp, const char *name) +{ +fprintf(llogfile, "%s={", name); +pprint_int16(pdu, rx, offsetp, "size"); +pprint_int16(pdu, rx, offsetp, ", type"); +pprint_int32(pdu, rx, offsetp, ", dev"); +pprint_qid(pdu, rx, offsetp, ", qid"); +pprint_int32(pdu, rx, offsetp, ", mode"); +pprint_int32(pdu, rx, offsetp, ", atime"); +pprint_int32(pdu, rx, offsetp, ", mtime"); +p
[Qemu-devel] [PATCH 08/17] virtio-9p: Implement P9_TCLUNK
This patch gets ls -al to work Signed-off-by: Anthony Liguori Signed-off-by: Aneesh Kumar K.V --- hw/virtio-9p.c | 16 ++-- 1 files changed, 14 insertions(+), 2 deletions(-) diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c index b30933a..7822a00 100644 --- a/hw/virtio-9p.c +++ b/hw/virtio-9p.c @@ -1305,8 +1305,20 @@ out: static void v9fs_clunk(V9fsState *s, V9fsPDU *pdu) { -if (debug_9p_pdu) - pprint_pdu(pdu); +int32_t fid; +size_t offset = 7; +int err; + +pdu_unmarshal(pdu, offset, "d", &fid); + +err = free_fid(s, fid); +if (err < 0) + goto out; + +offset = 7; +err = offset; +out: +complete_pdu(s, pdu, err); } static void v9fs_write(V9fsState *s, V9fsPDU *pdu) -- 1.6.5.2
[Qemu-devel] [PATCH 02/17] vrtio-9p: Implement P9_TVERSION for 9P
[ki...@linux.vnet.ibm.com: malloc to qemu_malloc coversion] Signed-off-by: Anthony Liguori Signed-off-by: Aneesh Kumar K.V --- hw/virtio-9p.c | 263 +++- 1 files changed, 262 insertions(+), 1 deletions(-) diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c index 93402c5..a057fbb 100644 --- a/hw/virtio-9p.c +++ b/hw/virtio-9p.c @@ -111,10 +111,271 @@ static void free_pdu(V9fsState *s, V9fsPDU *pdu) } } -static void v9fs_version(V9fsState *s, V9fsPDU *pdu) +static void v9fs_string_free(V9fsString *str) +{ +free(str->data); +str->data = NULL; +str->size = 0; +} + +static size_t pdu_unpack(void *dst, V9fsPDU *pdu, size_t offset, size_t size) +{ +struct iovec *sg = pdu->elem.out_sg; +BUG_ON((offset + size) > sg[0].iov_len); +memcpy(dst, sg[0].iov_base + offset, size); +return size; +} + +/* FIXME i can do this with less variables */ +static size_t pdu_pack(V9fsPDU *pdu, size_t offset, const void *src, size_t size) +{ +struct iovec *sg = pdu->elem.in_sg; +size_t off = 0; +size_t copied = 0; +int i = 0; + +for (i = 0; size && i < pdu->elem.in_num; i++) { + size_t len; + + if (offset >= off && offset < (off + sg[i].iov_len)) { + len = MIN(sg[i].iov_len - (offset - off), size); + memcpy(sg[i].iov_base + (offset - off), src, len); + size -= len; + offset += len; + off = offset; + copied += len; + src += len; + } else + off += sg[i].iov_len; +} + +return copied; +} + +static int pdu_copy_sg(V9fsPDU *pdu, size_t offset, int rx, struct iovec *sg) +{ +size_t pos = 0; +int i, j; +struct iovec *src_sg; +unsigned int num; + +if (rx) { + src_sg = pdu->elem.in_sg; + num = pdu->elem.in_num; +} else { + src_sg = pdu->elem.out_sg; + num = pdu->elem.out_num; +} + +j = 0; +for (i = 0; i < num; i++) { + if (offset <= pos) { + sg[j].iov_base = src_sg[i].iov_base; + sg[j].iov_len = src_sg[i].iov_len; + j++; + } else if (offset < (src_sg[i].iov_len + pos)) { + sg[j].iov_base = src_sg[i].iov_base; + sg[j].iov_len = src_sg[i].iov_len; + sg[j].iov_base += (offset - pos); + sg[j].iov_len -= (offset - pos); + j++; + } + pos += src_sg[i].iov_len; +} + +return j; +} + +static size_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...) +{ +size_t old_offset = offset; +va_list ap; +int i; + +va_start(ap, fmt); +for (i = 0; fmt[i]; i++) { + switch (fmt[i]) { + case 'b': { + int8_t *valp = va_arg(ap, int8_t *); + offset += pdu_unpack(valp, pdu, offset, sizeof(*valp)); + break; + } + case 'w': { + int16_t *valp = va_arg(ap, int16_t *); + offset += pdu_unpack(valp, pdu, offset, sizeof(*valp)); + break; + } + case 'd': { + int32_t *valp = va_arg(ap, int32_t *); + offset += pdu_unpack(valp, pdu, offset, sizeof(*valp)); + break; + } + case 'q': { + int64_t *valp = va_arg(ap, int64_t *); + offset += pdu_unpack(valp, pdu, offset, sizeof(*valp)); + break; + } + case 'v': { + struct iovec *iov = va_arg(ap, struct iovec *); + int *iovcnt = va_arg(ap, int *); + *iovcnt = pdu_copy_sg(pdu, offset, 0, iov); + break; + } + case 's': { + V9fsString *str = va_arg(ap, V9fsString *); + offset += pdu_unmarshal(pdu, offset, "w", &str->size); + /* FIXME: sanity check str->size */ + str->data = qemu_malloc(str->size + 1); + offset += pdu_unpack(str->data, pdu, offset, str->size); + str->data[str->size] = 0; + break; + } + case 'Q': { + V9fsQID *qidp = va_arg(ap, V9fsQID *); + offset += pdu_unmarshal(pdu, offset, "bdq", + &qidp->type, &qidp->version, &qidp->path); + break; + } + case 'S': { + V9fsStat *statp = va_arg(ap, V9fsStat *); + offset += pdu_unmarshal(pdu, offset, "wwdQdddqsddd", + &statp->size, &statp->type, &statp->dev, + &statp->qid, &statp->mode, &statp->atime, + &statp->mtime, &statp->length, + &statp->name, &statp->uid, &statp->gid, + &statp->muid, &statp->extension, + &statp->n_uid, &statp->n_gid, + &statp->n_muid); + break; + } + default: + break; + } +} + +va_end(ap); + +return offset - old_offset; +} + +static size_t pdu_marshal(V9fsPDU
[Qemu-devel] [PATCH 04/17] virtio-9p: Implement P9_TSTAT
This get the mount to work on the guest [ki...@linux.vnet.ibm.com: malloc to qemu_malloc conversion] Signed-off-by: Anthony Liguori Signed-off-by: Gautham R Shenoy Signed-off-by: Aneesh Kumar K.V --- hw/virtio-9p-local.c |7 ++ hw/virtio-9p.c | 169 +- 2 files changed, 174 insertions(+), 2 deletions(-) diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c index 204437c..9752f76 100644 --- a/hw/virtio-9p-local.c +++ b/hw/virtio-9p-local.c @@ -72,9 +72,16 @@ static int local_setuid(void *opaque, uid_t uid) return 0; } +static ssize_t local_readlink(void *opaque, const char *path, + char *buf, size_t bufsz) +{ +return readlink(rpath(path), buf, bufsz); +} + static V9fsPosixFileOperations ops = { .lstat = local_lstat, .setuid = local_setuid, +.readlink = local_readlink, }; V9fsPosixFileOperations *virtio_9p_init_local(const char *path) diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c index c63ac80..10bcd89 100644 --- a/hw/virtio-9p.c +++ b/hw/virtio-9p.c @@ -102,6 +102,21 @@ static int posix_setuid(V9fsState *s, uid_t uid) return s->ops->setuid(s->ops->opaque, uid); } +static ssize_t posix_readlink(V9fsState *s, V9fsString *path, V9fsString *buf) +{ +ssize_t len; + +buf->data = qemu_malloc(1024); + +len = s->ops->readlink(s->ops->opaque, path->data, buf->data, 1024 - 1); +if (len > -1) { + buf->size = len; + buf->data[len] = 0; +} + +return len; +} + static void v9fs_string_free(V9fsString *str) { free(str->data); @@ -109,6 +124,11 @@ static void v9fs_string_free(V9fsString *str) str->size = 0; } +static void v9fs_string_null(V9fsString *str) +{ +v9fs_string_free(str); +} + static void v9fs_string_sprintf(V9fsString *str, const char *fmt, ...) { va_list ap; @@ -124,6 +144,11 @@ static void v9fs_string_sprintf(V9fsString *str, const char *fmt, ...) str->size = err; } +static size_t v9fs_string_size(V9fsString *str) +{ +return str->size; +} + static V9fsFidState *lookup_fid(V9fsState *s, int32_t fid) { V9fsFidState *f; @@ -436,6 +461,15 @@ static size_t pdu_marshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...) return offset - old_offset; } +static void v9fs_stat_free(V9fsStat *stat) +{ +v9fs_string_free(&stat->name); +v9fs_string_free(&stat->uid); +v9fs_string_free(&stat->gid); +v9fs_string_free(&stat->muid); +v9fs_string_free(&stat->extension); +} + static void complete_pdu(V9fsState *s, V9fsPDU *pdu, ssize_t len) { int8_t id = pdu->id + 1; /* Response */ @@ -474,6 +508,88 @@ static void complete_pdu(V9fsState *s, V9fsPDU *pdu, ssize_t len) free_pdu(s, pdu); } +static uint32_t stat_to_v9mode(const struct stat *stbuf) +{ +uint32_t mode; + +mode = stbuf->st_mode & 0777; +if (S_ISDIR(stbuf->st_mode)) + mode |= P9_STAT_MODE_DIR; + +if (dotu) { + if (S_ISLNK(stbuf->st_mode)) + mode |= P9_STAT_MODE_SYMLINK; + if (S_ISSOCK(stbuf->st_mode)) + mode |= P9_STAT_MODE_SOCKET; + if (S_ISFIFO(stbuf->st_mode)) + mode |= P9_STAT_MODE_NAMED_PIPE; + if (S_ISBLK(stbuf->st_mode) || S_ISCHR(stbuf->st_mode)) + mode |= P9_STAT_MODE_DEVICE; + if (stbuf->st_mode & S_ISUID) + mode |= P9_STAT_MODE_SETUID; + if (stbuf->st_mode & S_ISGID) + mode |= P9_STAT_MODE_SETGID; + if (stbuf->st_mode & S_ISVTX) + mode |= P9_STAT_MODE_SETVTX; +} + +return mode; +} + +static void stat_to_v9stat(V9fsState *s, V9fsString *name, + const struct stat *stbuf, + V9fsStat *v9stat) +{ +int err; +const char *str; + +memset(v9stat, 0, sizeof(*v9stat)); + +stat_to_qid(stbuf, &v9stat->qid); +v9stat->mode = stat_to_v9mode(stbuf); +v9stat->atime = stbuf->st_atime; +v9stat->mtime = stbuf->st_mtime; +v9stat->length = stbuf->st_size; + +v9fs_string_null(&v9stat->uid); +v9fs_string_null(&v9stat->gid); +v9fs_string_null(&v9stat->muid); + +if (dotu) { + v9stat->n_uid = stbuf->st_uid; + v9stat->n_gid = stbuf->st_gid; + v9stat->n_muid = 0; + + v9fs_string_null(&v9stat->extension); + + if (v9stat->mode & P9_STAT_MODE_SYMLINK) { + err = posix_readlink(s, name, &v9stat->extension); + BUG_ON(err == -1); + v9stat->extension.data[err] = 0; + v9stat->extension.size = err; + } else if (v9stat->mode & P9_STAT_MODE_DEVICE) { + v9fs_string_sprintf(&v9stat->extension, "%c %u %u", + S_ISCHR(stbuf->st_mode) ? 'c' : 'b', + major(stbuf->st_rdev), minor(stbuf->st_rdev)); + } +} + +str = strrchr(name->data, '/'); +if (str) + str += 1; +else + str = name->data; + +v9fs_string_sprintf(&v9stat->name, "%s", str); + +v9
[Qemu-devel] [PATCH 10/17] virtio-9p: Implement P9_TCREATE
Signed-off-by: Anthony Liguori Signed-off-by: Gautham R Shenoy Signed-off-by: Aneesh Kumar K.V --- hw/virtio-9p-local.c | 80 +++ hw/virtio-9p.c | 267 +- 2 files changed, 345 insertions(+), 2 deletions(-) diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c index 441d22d..606f5be 100644 --- a/hw/virtio-9p-local.c +++ b/hw/virtio-9p-local.c @@ -135,6 +135,78 @@ static ssize_t local_writev(void *opaque, int fd, const struct iovec *iov, return writev(fd, iov, iovcnt); } +static int local_chmod(void *opaque, const char *path, mode_t mode) +{ +return chmod(rpath(path), mode); +} + +static int local_mknod(void *opaque, const char *path, mode_t mode, dev_t dev) +{ +return mknod(rpath(path), mode, dev); +} + +static int local_mksock(void *opaque, const char *path) +{ +struct sockaddr_un addr; +int s; + +addr.sun_family = AF_UNIX; +snprintf(addr.sun_path, 108, "%s", rpath(path)); + +s = socket(PF_UNIX, SOCK_STREAM, 0); +if (s == -1) + return -1; + +if (bind(s, (struct sockaddr *)&addr, sizeof(addr))) { + close(s); + return -1; +} + +close(s); +return 0; +} + +static int local_mkdir(void *opaque, const char *path, mode_t mode) +{ +return mkdir(rpath(path), mode); +} + +static int local_fstat(void *opaque, int fd, struct stat *stbuf) +{ +return fstat(fd, stbuf); +} + +static int local_open2(void *opaque, const char *path, int flags, mode_t mode) +{ +return open(rpath(path), flags, mode); +} + +static int local_symlink(void *opaque, const char *oldpath, +const char *newpath) +{ +return symlink(oldpath, rpath(newpath)); +} + +static int local_link(void *opaque, const char *oldpath, const char *newpath) +{ +char *tmp = strdup(rpath(oldpath)); +int err, serrno = 0; + +if (tmp == NULL) + return -ENOMEM; + +err = link(tmp, rpath(newpath)); +if (err == -1) + serrno = errno; + +free(tmp); + +if (err == -1) + errno = serrno; + +return err; +} + static V9fsPosixFileOperations ops = { .lstat = local_lstat, .setuid = local_setuid, @@ -150,6 +222,14 @@ static V9fsPosixFileOperations ops = { .readv = local_readv, .lseek = local_lseek, .writev = local_writev, +.chmod = local_chmod, +.mknod = local_mknod, +.mksock = local_mksock, +.mkdir = local_mkdir, +.fstat = local_fstat, +.open2 = local_open2, +.symlink = local_symlink, +.link = local_link, }; V9fsPosixFileOperations *virtio_9p_init_local(const char *path) diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c index b0662ba..841fcde 100644 --- a/hw/virtio-9p.c +++ b/hw/virtio-9p.c @@ -174,6 +174,47 @@ static int posix_writev(V9fsState *s, int fd, const struct iovec *iov, return s->ops->writev(s->ops->opaque, fd, iov, iovcnt); } +static int posix_chmod(V9fsState *s, V9fsString *path, mode_t mode) +{ +return s->ops->chmod(s->ops->opaque, path->data, mode); +} + +static int posix_mknod(V9fsState *s, V9fsString *path, mode_t mode, dev_t dev) +{ +return s->ops->mknod(s->ops->opaque, path->data, mode, dev); +} + +static int posix_mksock(V9fsState *s, V9fsString *path) +{ +return s->ops->mksock(s->ops->opaque, path->data); +} + +static int posix_mkdir(V9fsState *s, V9fsString *path, mode_t mode) +{ +return s->ops->mkdir(s->ops->opaque, path->data, mode); +} + +static int posix_fstat(V9fsState *s, int fd, struct stat *stbuf) +{ +return s->ops->fstat(s->ops->opaque, fd, stbuf); +} + +static int posix_open2(V9fsState *s, V9fsString *path, int flags, mode_t mode) +{ +return s->ops->open2(s->ops->opaque, path->data, flags, mode); +} + +static int posix_symlink(V9fsState *s, V9fsString *oldpath, +V9fsString *newpath) +{ +return s->ops->symlink(s->ops->opaque, oldpath->data, newpath->data); +} + +static int posix_link(V9fsState *s, V9fsString *oldpath, V9fsString *newpath) +{ +return s->ops->link(s->ops->opaque, oldpath->data, newpath->data); +} + static void v9fs_string_init(V9fsString *str) { str->data = NULL; @@ -1420,10 +1461,232 @@ out: qemu_free(vs); } +typedef struct V9fsCreateState { +V9fsPDU *pdu; +size_t offset; +int32_t fid; +V9fsFidState *fidp; +V9fsQID qid; +int32_t perm; +int8_t mode; +struct stat stbuf; +V9fsString name; +V9fsString extension; +V9fsString fullname; +} V9fsCreateState; + +static void v9fs_post_create(V9fsState *s, V9fsCreateState *vs, int err) +{ +if (err == 0) { +v9fs_string_copy(&vs->fidp->path, &vs->fullname); +stat_to_qid(&vs->stbuf, &vs->qid); + +vs->offset += pdu_marshal(vs->pdu, vs->offset, "Qd", &vs->qid, 0); + +err = vs->offset; +} + +complete_pdu(s, vs->pdu, err); +v9fs_string_free(&vs->name); +v9fs_string_free(&vs->extension); +v9fs_string_free(&vs->fullname); +qemu_free(vs); +} +
[Qemu-devel] Re: [PATCH 05/20] eepro100: Add all supported devices to pci.c
Michael S. Tsirkin schrieb: > On Wed, Mar 03, 2010 at 02:31:53PM +0100, Gerd Hoffmann wrote: >> On 03/03/10 12:51, Michael S. Tsirkin wrote: >>> On Sun, Feb 21, 2010 at 10:08:49PM +0100, Stefan Weil wrote: >> static const char * const pci_nic_models[] = { >> "ne2k_pci", >> + "i82550", >> "i82551", >> + "i82557a", >> "i82557b", >> + "i82557c", >> + "i82558a", >> + "i82558b", >> + "i82559a", >> + "i82559b", >> + "i82559c", >> "i82559er", >> + "i82562", >> "rtl8139", >> "e1000", >> "pcnet", > One wonders: would it be cleaner to have a single eepro100 device > with specific model as qdev option? >> No. I think it would be cleaner to use qdev even more. For that >> specific driver it would make sense to create a private Info struct, >> i.e. something like this: >> >> E100PCIDeviceInfo { >> PCIDeviceInfo pci; >> [ model specific stuff such as has_extended_tcb_support here ] >> }; >> >> Then go >> >> static E100PCIDeviceInfo eepro100_info[] = { >> { >> .pci.qdev.name = "i82550", >> [ usual qdev stuff here ] >> has_extended_tcb_support = 0 | 1; >> [ more e100 device description bits here ] >> }, >> [ ... ] >> }; >> >> Then you can simply zap all the one-liner wrapper functions around >> nic_init(). nic_init() can get a pointer using something like this: >> >> E100PCIDeviceInfo *einfo = DO_UPCAST(E100PCIDeviceInfo, >> pci.qdev, pci_dev->qdev.info); >> >> and setup the device accordingly. This would indeed simplify the code. Although I don't like tricky programming like DO_UPCAST (because I saw too much code where tricky programming was faulty programming), I'll consider to modify the code as Gerd proposed. I prefer the "official" device names which are also used in the Intel documentation. eepro100 or e100 are marketing names (more of them exist). >> qdev has aliases. You can add .qdev.alias = "eepro100" to one of the >> variants, then the eepro100 name will work as well. Good idea. To keep the number of aliases small, I plan to add only one or two aliases: e100 (eepro100 only if a second alias is possible). Why? e100 is the current linux driver name. eepro100 was the former linux driver name. >> >> Adding the marketing name to the .desc test is probably a good idea too >> because alot of people know the marketing name only. > > That's my thinking as well. Neither eepro100 nor e100 are marketing names (I was wrong when I wrote that in an earlier mail). The marketing name was Intel EtherExpress Pro 100. I agree that it would help people to see this name, so I suggest adding it to qemu-doc.texi. As far as I know, only insiders (and the old linux driver) used the abbreviations eepro100 and e100. Look at your favorite search machine: nobody sells / sold eepro100 cards. If you want a less technical name than i82559c, e100 is the better choice - like e1000 for the successor. > Please note that this patch was marked as optional. The arrays pci_nic_names and pci_nic_models are not really needed, and as soon as the table of available nics is automatically derived from the device information, all variants of i825xx are supported automatically. >>> Gerd, any input on this patch? >> Looks fine to me. When we switch over to walking the list of registered >> devices instead of having a hard-coded list of pci nic drivers all the >> eepro100 variants will show up in the list anyway. >> >> cheers, >> Gerd
[Qemu-devel] Re: [PATCH] configure: Fix wrong stderr redirection
Stefan Weil wrote: > Signed-off-by: Stefan Weil Ouch. good catch. Acked-by: Juan Quintela > --- > configure |4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/configure b/configure > index dfe3e42..a14aba7 100755 > --- a/configure > +++ b/configure > @@ -1091,8 +1091,8 @@ EOF > # static link with sdl ? (note: sdl.pc's --static --libs is broken) > if test "$sdl" = "yes" -a "$static" = "yes" ; then >if test $? = 0 && echo $sdl_libs | grep -- -laa > /dev/null; then > - sdl_libs="$sdl_libs `aalib-config --static-libs >2 /dev/null`" > - sdl_cflags="$sdl_cflags `aalib-config --cflags >2 /dev/null`" > + sdl_libs="$sdl_libs `aalib-config --static-libs 2>/dev/null`" > + sdl_cflags="$sdl_cflags `aalib-config --cflags 2>/dev/null`" >fi >if compile_prog "$sdl_cflags" "$sdl_libs" ; then > :
Re: [Qemu-devel] Possible invalid emulation of rex.W-prefixed far jump
3On Mon, 1 Mar 2010, Brad Spengler wrote: > Hi all, > > I'm writing to report a possible bug in the qemu emulation of > rex.W-prefixed far jumps. It affects far jumps of this type with both > rip-relative and absolute addresses. Looks like it's a bug indeed, here's a test case: .data #define REX #ifdef REX jmp_off:.quad longproc #else jmp_off:.long longproc #endif jmp_seg:.long 0 msg0: .ascii "Hello\n" msg1: .ascii "Jump target reached\n" msg2: .ascii "Bye\n" msg3: .text .globl _start show: xor %rax,%rax inc %rax mov %rax,%rdi syscall retq _start: lea msg0, %rsi mov $(msg1-msg0), %rdx call show mov %cs, jmp_seg #ifdef REX .byte 0x48 #endif ljmp *jmp_off back: lea msg2, %rsi mov $(msg3-msg2), %rdx call show mov $60, %rax syscall longproc: lea msg1, %rsi mov $(msg2-msg1), %rdx call show jmp back To build: gcc -m64 -o test -nostartfiles test.S [-DREX] > > The yasm syntax for these instructions: > jmp far qword [addr] > jmp far qword [addr wrt rip] > > and the resulting disassembly: > 8: 48 ff 2c 25 00 00 00 00 rex.W ljmpq *0x0 c: R_X86_64_32 .text+0x17 > 10: 48 ff 2d 00 00 00 00rex.W ljmpq *0x0(%rip)# 0x17 > > qemu triggers a gpf with error 0xfffc (presumably this is 0x masked > to 0xfffc by the & on new_cs from > target-i386/op_helper.c:helper_ljmp_protected()) > > It's suspected that qemu is treating the far address as 16:32 instead of > 16:64 as it should, since the far address as laid out in memory is: > 12 34 56 78 ff ff ff ff 10 00 > > The far address is intended to be a Linux kernel address, so the upper > 32bits are 0x. > > If qemu is treating the 16:64 layout as 16:32, you can see why new_cs > would have the value of 0x. The code only fails on qemu -- it works > as expected on real systems. > > I'm not familiar with your code-base so I have no patch for the issue, > but I thought I'd fire off a mail as I imagine it's a simple > oversight and easy fix for someone familiar with the code. > The code in translate.c (line 4573 and bellow) doesn't even look at REXW. > Thanks for your help, and please keep me on CC for replies Thanks for the bug report. -- mailto:av1...@comtv.ru
RE: [Qemu-devel] Intel PXA270 System-on-chip emulation in QEMU
Hi, Can you tell me name of any platform which is emulated in QEMU and I can run my USB device application on it? I mean I want to have board(emulated in QEMU) behaving as a mass storage device (running mass storage application). -Taimoor > Date: Tue, 2 Mar 2010 23:52:18 +0100 > Subject: Re: [Qemu-devel] Intel PXA270 System-on-chip emulation in QEMU > From: balr...@gmail.com > To: mooni_mi...@hotmail.com > CC: qemu-devel@nongnu.org > > Hi Taimoor, > > On 1 March 2010 09:01, Taimoor Mirza wrote: > > I want to know about Intel PXA270 System-on-chip emulation in QEMU. Does > > this emulation includes "USB client controller" emulation? > > If you mean usb "slave" or "device" support, no, it's not supported by > Qemu's PXA270 emulator. There is a framework for this used by another > System-on-chip in qemu, so it can be implemented for PXA270 too. > > Cheers > > _ Your E-mail and More On-the-Go. Get Windows Live Hotmail Free. https://signup.live.com/signup.aspx?id=60969
[Qemu-devel] [PATCHv4 12/12] virtio-net: vhost net support
This connects virtio-net to vhost net backend. The code is structured in a way analogous to what we have with vnet header capability in tap. We start/stop backend on driver start/stop as well as on save and vm start (for migration). Signed-off-by: Michael S. Tsirkin --- hw/virtio-net.c | 71 +- 1 files changed, 69 insertions(+), 2 deletions(-) diff --git a/hw/virtio-net.c b/hw/virtio-net.c index 5c0093e..9ddd58c 100644 --- a/hw/virtio-net.c +++ b/hw/virtio-net.c @@ -17,6 +17,7 @@ #include "net/tap.h" #include "qemu-timer.h" #include "virtio-net.h" +#include "vhost_net.h" #define VIRTIO_NET_VM_VERSION11 @@ -47,6 +48,8 @@ typedef struct VirtIONet uint8_t nomulti; uint8_t nouni; uint8_t nobcast; +uint8_t vhost_started; +VMChangeStateEntry *vmstate; struct { int in_use; int first_multi; @@ -114,6 +117,10 @@ static void virtio_net_reset(VirtIODevice *vdev) n->nomulti = 0; n->nouni = 0; n->nobcast = 0; +if (n->vhost_started) { +vhost_net_stop(tap_get_vhost_net(n->nic->nc.peer), vdev); +n->vhost_started = 0; +} /* Flush any MAC and VLAN filter table state */ n->mac_table.in_use = 0; @@ -172,7 +179,14 @@ static uint32_t virtio_net_get_features(VirtIODevice *vdev, uint32_t features) features &= ~(0x1 << VIRTIO_NET_F_HOST_UFO); } -return features; +if (!n->nic->nc.peer || +n->nic->nc.peer->info->type != NET_CLIENT_TYPE_TAP) { +return features; +} +if (!tap_get_vhost_net(n->nic->nc.peer)) { +return features; +} +return vhost_net_get_features(tap_get_vhost_net(n->nic->nc.peer), features); } static uint32_t virtio_net_bad_features(VirtIODevice *vdev) @@ -698,6 +712,12 @@ static void virtio_net_save(QEMUFile *f, void *opaque) { VirtIONet *n = opaque; +if (n->vhost_started) { +/* TODO: should we really stop the backend? + * If we don't, it might keep writing to memory. */ +vhost_net_stop(tap_get_vhost_net(n->nic->nc.peer), &n->vdev); +n->vhost_started = 0; +} virtio_save(&n->vdev, f); qemu_put_buffer(f, n->mac, ETH_ALEN); @@ -810,7 +830,6 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id) qemu_mod_timer(n->tx_timer, qemu_get_clock(vm_clock) + TX_TIMER_INTERVAL); } - return 0; } @@ -830,6 +849,47 @@ static NetClientInfo net_virtio_info = { .link_status_changed = virtio_net_set_link_status, }; +static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status) +{ +VirtIONet *n = to_virtio_net(vdev); +if (!n->nic->nc.peer) { +return; +} +if (n->nic->nc.peer->info->type != NET_CLIENT_TYPE_TAP) { +return; +} + +if (!tap_get_vhost_net(n->nic->nc.peer)) { +return; +} +if (!!n->vhost_started == !!(status & VIRTIO_CONFIG_S_DRIVER_OK)) { +return; +} +if (status & VIRTIO_CONFIG_S_DRIVER_OK) { +int r = vhost_net_start(tap_get_vhost_net(n->nic->nc.peer), vdev); +if (r < 0) { +fprintf(stderr, "unable to start vhost net: %d: " +"falling back on userspace virtio\n", -r); +} else { +n->vhost_started = 1; +} +} else { +vhost_net_stop(tap_get_vhost_net(n->nic->nc.peer), vdev); +n->vhost_started = 0; +} +} + +static void virtio_net_vmstate_change(void *opaque, int running, int reason) +{ +VirtIONet *n = opaque; +if (!running) { +return; +} +/* This is called when vm is started, it will start vhost backend if + * appropriate e.g. after migration. */ +virtio_net_set_status(&n->vdev, n->vdev.status); +} + VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf) { VirtIONet *n; @@ -845,6 +905,7 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf) n->vdev.set_features = virtio_net_set_features; n->vdev.bad_features = virtio_net_bad_features; n->vdev.reset = virtio_net_reset; +n->vdev.set_status = virtio_net_set_status; n->rx_vq = virtio_add_queue(&n->vdev, 256, virtio_net_handle_rx); n->tx_vq = virtio_add_queue(&n->vdev, 256, virtio_net_handle_tx); n->ctrl_vq = virtio_add_queue(&n->vdev, 64, virtio_net_handle_ctrl); @@ -867,6 +928,7 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf) register_savevm("virtio-net", virtio_net_id++, VIRTIO_NET_VM_VERSION, virtio_net_save, virtio_net_load, n); +n->vmstate = qemu_add_vm_change_state_handler(virtio_net_vmstate_change, n); return &n->vdev; } @@ -874,6 +936,11 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf) void virtio_net_exit(VirtIODevice *vdev) { VirtIONet *n = DO_UPCAST(VirtIONet, vdev, vdev); +qemu_del_vm_change_state_handler(n->vmstate); + +if (n->vhost_started) { +
[Qemu-devel] [PATCHv4 11/12] tap: add API to retrieve vhost net header
will be used by virtio-net for vhost net support Signed-off-by: Michael S. Tsirkin --- net/tap.c |7 +++ net/tap.h |3 +++ 2 files changed, 10 insertions(+), 0 deletions(-) diff --git a/net/tap.c b/net/tap.c index 19c4fa2..35c05d7 100644 --- a/net/tap.c +++ b/net/tap.c @@ -487,3 +487,10 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan return 0; } + +VHostNetState *tap_get_vhost_net(VLANClientState *nc) +{ +TAPState *s = DO_UPCAST(TAPState, nc, nc); +assert(nc->info->type == NET_CLIENT_TYPE_TAP); +return s->vhost_net; +} diff --git a/net/tap.h b/net/tap.h index a244b28..b8cec83 100644 --- a/net/tap.h +++ b/net/tap.h @@ -50,4 +50,7 @@ void tap_fd_set_offload(int fd, int csum, int tso4, int tso6, int ecn, int ufo); int tap_get_fd(VLANClientState *vc); +struct vhost_net; +struct vhost_net *tap_get_vhost_net(VLANClientState *vc); + #endif /* QEMU_NET_TAP_H */ -- 1.7.0.18.g0d53a5
[Qemu-devel] [PATCHv4 10/12] tap: add vhost/vhostfd options
This adds vhost binary option to tap, to enable vhost net accelerator. Default is off for now, we'll be able to make default on long term when we know it's stable. vhostfd option can be used by management, to pass in the fd. Assigning vhostfd implies vhost=on. Signed-off-by: Michael S. Tsirkin --- net.c |8 net/tap.c | 29 + qemu-options.hx |4 +++- 3 files changed, 40 insertions(+), 1 deletions(-) diff --git a/net.c b/net.c index a1bf49f..d1e23f1 100644 --- a/net.c +++ b/net.c @@ -973,6 +973,14 @@ static const struct { .name = "vnet_hdr", .type = QEMU_OPT_BOOL, .help = "enable the IFF_VNET_HDR flag on the tap interface" +}, { +.name = "vhost", +.type = QEMU_OPT_BOOL, +.help = "enable vhost-net network accelerator", +}, { +.name = "vhostfd", +.type = QEMU_OPT_STRING, +.help = "file descriptor of an already opened vhost net device", }, #endif /* _WIN32 */ { /* end of list */ } diff --git a/net/tap.c b/net/tap.c index fc59fd4..19c4fa2 100644 --- a/net/tap.c +++ b/net/tap.c @@ -41,6 +41,8 @@ #include "net/tap-linux.h" +#include "hw/vhost_net.h" + /* Maximum GSO packet size (64k) plus plenty of room for * the ethernet and virtio_net headers */ @@ -57,6 +59,7 @@ typedef struct TAPState { unsigned int has_vnet_hdr : 1; unsigned int using_vnet_hdr : 1; unsigned int has_ufo: 1; +VHostNetState *vhost_net; } TAPState; static int launch_script(const char *setup_script, const char *ifname, int fd); @@ -252,6 +255,10 @@ static void tap_cleanup(VLANClientState *nc) { TAPState *s = DO_UPCAST(TAPState, nc, nc); +if (s->vhost_net) { +vhost_net_cleanup(s->vhost_net); +} + qemu_purge_queued_packets(nc); if (s->down_script[0]) @@ -307,6 +314,7 @@ static TAPState *net_tap_fd_init(VLANState *vlan, s->has_ufo = tap_probe_has_ufo(s->fd); tap_set_offload(&s->nc, 0, 0, 0, 0, 0); tap_read_poll(s, 1); +s->vhost_net = NULL; return s; } @@ -456,5 +464,26 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan } } +if (qemu_opt_get_bool(opts, "vhost", !!qemu_opt_get(opts, "vhostfd"))) { +int vhostfd, r; +if (qemu_opt_get(opts, "vhostfd")) { +r = net_handle_fd_param(mon, qemu_opt_get(opts, "vhostfd")); +if (r == -1) { +return -1; +} +vhostfd = r; +} else { +vhostfd = -1; +} +s->vhost_net = vhost_net_init(&s->nc, vhostfd); +if (!s->vhost_net) { +qemu_error("vhost-net requested but could not be initialized\n"); +return -1; +} +} else if (qemu_opt_get(opts, "vhostfd")) { +qemu_error("vhostfd= is not valid without vhost\n"); +return -1; +} + return 0; } diff --git a/qemu-options.hx b/qemu-options.hx index 7daa246..9dc81b4 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -879,7 +879,7 @@ DEF("net", HAS_ARG, QEMU_OPTION_net, "-net tap[,vlan=n][,name=str],ifname=name\n" "connect the host TAP network interface to VLAN 'n'\n" #else -"-net tap[,vlan=n][,name=str][,fd=h][,ifname=name][,script=file][,downscript=dfile][,sndbuf=nbytes][,vnet_hdr=on|off]\n" +"-net tap[,vlan=n][,name=str][,fd=h][,ifname=name][,script=file][,downscript=dfile][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off][,vhostfd=h]\n" "connect the host TAP network interface to VLAN 'n' and use the\n" "network scripts 'file' (default=" DEFAULT_NETWORK_SCRIPT ")\n" "and 'dfile' (default=" DEFAULT_NETWORK_DOWN_SCRIPT ")\n" @@ -889,6 +889,8 @@ DEF("net", HAS_ARG, QEMU_OPTION_net, "default of 'sndbuf=1048576' can be disabled using 'sndbuf=0')\n" "use vnet_hdr=off to avoid enabling the IFF_VNET_HDR tap flag\n" "use vnet_hdr=on to make the lack of IFF_VNET_HDR support an error condition\n" +"use vhost=on to enable experimental in kernel accelerator\n" +"use 'vhostfd=h' to connect to an already opened vhost net device\n" #endif "-net socket[,vlan=n][,name=str][,fd=h][,listen=[host]:port][,connect=host:port]\n" "connect the vlan 'n' to another VLAN using a socket connection\n" -- 1.7.0.18.g0d53a5
[Qemu-devel] [PATCHv4 09/12] vhost: vhost net support
This adds vhost net device support in qemu. Will be tied to tap device and virtio by following patches. Raw backend is currently missing, will be worked on/submitted separately. Signed-off-by: Michael S. Tsirkin --- Makefile.target |2 + configure | 36 +++ hw/vhost.c | 706 +++ hw/vhost.h | 48 hw/vhost_net.c | 198 hw/vhost_net.h | 19 ++ 6 files changed, 1009 insertions(+), 0 deletions(-) create mode 100644 hw/vhost.c create mode 100644 hw/vhost.h create mode 100644 hw/vhost_net.c create mode 100644 hw/vhost_net.h diff --git a/Makefile.target b/Makefile.target index 1f5aa9d..f833735 100644 --- a/Makefile.target +++ b/Makefile.target @@ -173,6 +173,8 @@ obj-y = vl.o async.o monitor.o pci.o pci_host.o pcie_host.o machine.o gdbstub.o # need to fix this properly obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-pci.o virtio-serial-bus.o obj-y += notifier.o +obj-y += vhost_net.o +obj-$(CONFIG_VHOST_NET) += vhost.o obj-y += rwhandler.o obj-$(CONFIG_KVM) += kvm.o kvm-all.o obj-$(CONFIG_ISA_MMIO) += isa_mmio.o diff --git a/configure b/configure index 8eb5f5b..66f0335 100755 --- a/configure +++ b/configure @@ -87,6 +87,7 @@ libs_softmmu="" libs_tools="" audio_pt_int="" audio_win_int="" +audio_win_int="" # parse CC options first for opt do @@ -263,6 +264,7 @@ vnc_tls="" vnc_sasl="" xen="" linux_aio="" +vhost_net="" gprof="no" debug_tcg="no" @@ -651,6 +653,10 @@ for opt do ;; --enable-docs) docs="yes" ;; + --disable-vhost-net) vhost_net="no" + ;; + --enable-vhost-net) vhost_net="yes" + ;; *) echo "ERROR: unknown option $opt"; show_help="yes" ;; esac @@ -1498,6 +1504,32 @@ EOF fi ## +# test for vhost net + +if test "$vhost_net" != "no"; then +if test "$kvm" != "no"; then +cat > $TMPC < +int main(void) { return 0; } +EOF +if compile_prog "$kvm_cflags" "" ; then +vhost_net=yes +else +if "$vhost_net" == "yes" ; then +feature_not_found "vhost-net" +fi +vhost_net=no +fi +else +if "$vhost_net" == "yes" ; then +echo -e "NOTE: vhost-net feature requires KVM (--enable-kvm)." +feature_not_found "vhost-net" +fi +vhost_net=no +fi +fi + +## # pthread probe PTHREADLIBS_LIST="-lpthread -lpthreadGC2" @@ -1968,6 +2000,7 @@ echo "fdt support $fdt" echo "preadv support$preadv" echo "fdatasync $fdatasync" echo "uuid support $uuid" +echo "vhost-net support $vhost_net" if test $sdl_too_old = "yes"; then echo "-> Your SDL version is too old - please upgrade to have SDL support" @@ -2492,6 +2525,9 @@ case "$target_arch2" in if test "$kvm_para" = "yes"; then echo "CONFIG_KVM_PARA=y" >> $config_target_mak fi + if test $vhost_net = "yes" ; then +echo "CONFIG_VHOST_NET=y" >> $config_target_mak + fi fi esac echo "TARGET_PHYS_ADDR_BITS=$target_phys_bits" >> $config_target_mak diff --git a/hw/vhost.c b/hw/vhost.c new file mode 100644 index 000..12a5757 --- /dev/null +++ b/hw/vhost.c @@ -0,0 +1,706 @@ +/* + * vhost support + * + * Copyright Red Hat, Inc. 2010 + * + * Authors: + * Michael S. Tsirkin + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + */ + +#include +#include +#include +#include "vhost.h" +#include "hw/hw.h" +/* For range_get_last */ +#include "pci.h" + +static void vhost_dev_sync_region(struct vhost_dev *dev, + uint64_t mfirst, uint64_t mlast, + uint64_t rfirst, uint64_t rlast) +{ +uint64_t start = MAX(mfirst, rfirst); +uint64_t end = MIN(mlast, rlast); +vhost_log_chunk_t *from = dev->log + start / VHOST_LOG_CHUNK; +vhost_log_chunk_t *to = dev->log + end / VHOST_LOG_CHUNK + 1; +uint64_t addr = (start / VHOST_LOG_CHUNK) * VHOST_LOG_CHUNK; + +assert(end / VHOST_LOG_CHUNK < dev->log_size); +assert(start / VHOST_LOG_CHUNK < dev->log_size); +if (end < start) { +return; +} +for (;from < to; ++from) { +vhost_log_chunk_t log; +int bit; +/* We first check with non-atomic: much cheaper, + * and we expect non-dirty to be the common case. */ +if (!*from) { +continue; +} +/* Data must be read atomically. We don't really + * need the barrier semantics of __sync + * builtins, but it's easier to use them than + * roll our own. */ +log = __sync_fetch_and_and(from, 0); +while ((bit = sizeof(log) > sizeof(int) ? +ffsll(log) : ffs(log))) { +bit -= 1; +cpu_physical_memory_
[Qemu-devel] [PATCHv4 08/12] virtio-pci: fill in notifier support
Support host/guest notifiers in virtio-pci. The last one only with kvm, that's okay because vhost relies on kvm anyway. Note on kvm usage: kvm ioeventfd API is implemented on non-kvm systems as well, this is the reason we don't need if (kvm_enabled()) around it. Signed-off-by: Michael S. Tsirkin --- hw/virtio-pci.c | 62 +++ 1 files changed, 62 insertions(+), 0 deletions(-) diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index fd9759a..596f056 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -24,6 +24,7 @@ #include "net.h" #include "block_int.h" #include "loader.h" +#include "kvm.h" /* from Linux's linux/virtio_pci.h */ @@ -392,6 +393,65 @@ static unsigned virtio_pci_get_features(void *opaque) return proxy->host_features; } +static void virtio_pci_guest_notifier_read(void *opaque) +{ +VirtQueue *vq = opaque; +EventNotifier *n = virtio_queue_guest_notifier(vq); +if (event_notifier_test_and_clear(n)) { +virtio_irq(vq); +} +} + +static int virtio_pci_guest_notifier(void *opaque, int n, bool assign) +{ +VirtIOPCIProxy *proxy = opaque; +VirtQueue *vq = virtio_queue(proxy->vdev, n); +EventNotifier *notifier = virtio_queue_guest_notifier(vq); + +if (assign) { +int r = event_notifier_init(notifier, 0); + if (r < 0) + return r; +qemu_set_fd_handler(event_notifier_get_fd(notifier), +virtio_pci_guest_notifier_read, NULL, vq); +} else { +qemu_set_fd_handler(event_notifier_get_fd(notifier), +NULL, NULL, NULL); +event_notifier_cleanup(notifier); +} + +return 0; +} + +static int virtio_pci_host_notifier(void *opaque, int n, bool assign) +{ +VirtIOPCIProxy *proxy = opaque; +VirtQueue *vq = virtio_queue(proxy->vdev, n); +EventNotifier *notifier = virtio_queue_host_notifier(vq); +int r; +if (assign) { +r = event_notifier_init(notifier, 1); +if (r < 0) { +return r; +} +r = kvm_set_ioeventfd_pio_word(event_notifier_get_fd(notifier), + proxy->addr + VIRTIO_PCI_QUEUE_NOTIFY, + n, assign); +if (r < 0) { +event_notifier_cleanup(notifier); +} +} else { +r = kvm_set_ioeventfd_pio_word(event_notifier_get_fd(notifier), + proxy->addr + VIRTIO_PCI_QUEUE_NOTIFY, + n, assign); +if (r < 0) { +return r; +} +event_notifier_cleanup(notifier); +} +return r; +} + static const VirtIOBindings virtio_pci_bindings = { .notify = virtio_pci_notify, .save_config = virtio_pci_save_config, @@ -399,6 +459,8 @@ static const VirtIOBindings virtio_pci_bindings = { .save_queue = virtio_pci_save_queue, .load_queue = virtio_pci_load_queue, .get_features = virtio_pci_get_features, +.host_notifier = virtio_pci_host_notifier, +.guest_notifier = virtio_pci_guest_notifier, }; static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev, -- 1.7.0.18.g0d53a5
[Qemu-devel] [PATCHv4 07/12] virtio: move typedef to qemu-common
make it possible to use type without header include Signed-off-by: Michael S. Tsirkin --- hw/virtio.h |1 - qemu-common.h |1 + 2 files changed, 1 insertions(+), 1 deletions(-) diff --git a/hw/virtio.h b/hw/virtio.h index 58b06bf..6f2fab0 100644 --- a/hw/virtio.h +++ b/hw/virtio.h @@ -69,7 +69,6 @@ static inline target_phys_addr_t vring_align(target_phys_addr_t addr, } typedef struct VirtQueue VirtQueue; -typedef struct VirtIODevice VirtIODevice; #define VIRTQUEUE_MAX_SIZE 1024 diff --git a/qemu-common.h b/qemu-common.h index f12a8f5..90ca3b8 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -228,6 +228,7 @@ typedef struct I2SCodec I2SCodec; typedef struct DeviceState DeviceState; typedef struct SSIBus SSIBus; typedef struct EventNotifier EventNotifier; +typedef struct VirtIODevice VirtIODevice; typedef uint64_t pcibus_t; -- 1.7.0.18.g0d53a5
[Qemu-devel] [PATCHv4 06/12] virtio: add set_status callback
vhost net backend needs to be notified when frontend status changes. Add a callback, similar to set_features. Signed-off-by: Michael S. Tsirkin --- hw/s390-virtio-bus.c |2 +- hw/syborg_virtio.c |2 +- hw/virtio-pci.c |5 +++-- hw/virtio.h |9 + 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c index fa0a74f..6f93d67 100644 --- a/hw/s390-virtio-bus.c +++ b/hw/s390-virtio-bus.c @@ -242,7 +242,7 @@ void s390_virtio_device_update_status(VirtIOS390Device *dev) VirtIODevice *vdev = dev->vdev; uint32_t features; -vdev->status = ldub_phys(dev->dev_offs + VIRTIO_DEV_OFFS_STATUS); +virtio_set_status(vdev, ldub_phys(dev->dev_offs + VIRTIO_DEV_OFFS_STATUS)); /* Update guest supported feature bitmap */ diff --git a/hw/syborg_virtio.c b/hw/syborg_virtio.c index 65239a0..c7b1162 100644 --- a/hw/syborg_virtio.c +++ b/hw/syborg_virtio.c @@ -149,7 +149,7 @@ static void syborg_virtio_writel(void *opaque, target_phys_addr_t offset, virtio_queue_notify(vdev, value); break; case SYBORG_VIRTIO_STATUS: -vdev->status = value & 0xFF; +virtio_set_status(vdev, value); if (vdev->status == 0) virtio_reset(vdev); break; diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index bcd40f7..fd9759a 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -206,7 +206,7 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val) virtio_queue_notify(vdev, val); break; case VIRTIO_PCI_STATUS: -vdev->status = val & 0xFF; +virtio_set_status(vdev, val & 0xFF); if (vdev->status == 0) { virtio_reset(proxy->vdev); msix_unuse_all_vectors(&proxy->pci_dev); @@ -377,7 +377,8 @@ static void virtio_write_config(PCIDevice *pci_dev, uint32_t address, if (PCI_COMMAND == address) { if (!(val & PCI_COMMAND_MASTER)) { -proxy->vdev->status &= ~VIRTIO_CONFIG_S_DRIVER_OK; +virtio_set_status(proxy->vdev, + proxy->vdev->status & ~VIRTIO_CONFIG_S_DRIVER_OK); } } diff --git a/hw/virtio.h b/hw/virtio.h index 26cf5fd..58b06bf 100644 --- a/hw/virtio.h +++ b/hw/virtio.h @@ -115,12 +115,21 @@ struct VirtIODevice void (*get_config)(VirtIODevice *vdev, uint8_t *config); void (*set_config)(VirtIODevice *vdev, const uint8_t *config); void (*reset)(VirtIODevice *vdev); +void (*set_status)(VirtIODevice *vdev, uint8_t val); VirtQueue *vq; const VirtIOBindings *binding; void *binding_opaque; uint16_t device_id; }; +static inline void virtio_set_status(VirtIODevice *vdev, uint8_t val) +{ +if (vdev->set_status) { +vdev->set_status(vdev, val); +} +vdev->status = val; +} + VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size, void (*handle_output)(VirtIODevice *, VirtQueue *)); -- 1.7.0.18.g0d53a5
[Qemu-devel] [PATCHv4 03/12] notifier: event notifier implementation
event notifiers are slightly generalized eventfd descriptors. Current implementation depends on eventfd because vhost is the only user, and vhost depends on eventfd anyway, but a stub is provided for non-eventfd case. We'll be able to further generalize this when another user comes along and we see how to best do this. Signed-off-by: Michael S. Tsirkin --- Makefile.target |1 + hw/notifier.c | 62 +++ hw/notifier.h | 16 ++ qemu-common.h |1 + 4 files changed, 80 insertions(+), 0 deletions(-) create mode 100644 hw/notifier.c create mode 100644 hw/notifier.h diff --git a/Makefile.target b/Makefile.target index 320f807..1f5aa9d 100644 --- a/Makefile.target +++ b/Makefile.target @@ -172,6 +172,7 @@ obj-y = vl.o async.o monitor.o pci.o pci_host.o pcie_host.o machine.o gdbstub.o # virtio has to be here due to weird dependency between PCI and virtio-net. # need to fix this properly obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-pci.o virtio-serial-bus.o +obj-y += notifier.o obj-y += rwhandler.o obj-$(CONFIG_KVM) += kvm.o kvm-all.o obj-$(CONFIG_ISA_MMIO) += isa_mmio.o diff --git a/hw/notifier.c b/hw/notifier.c new file mode 100644 index 000..c6db3c3 --- /dev/null +++ b/hw/notifier.c @@ -0,0 +1,62 @@ +/* + * event notifier support + * + * Copyright Red Hat, Inc. 2010 + * + * Authors: + * Michael S. Tsirkin + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + */ + +#include "hw.h" +#include "notifier.h" +#ifdef CONFIG_EVENTFD +#include +#endif + +int event_notifier_init(EventNotifier *e, int active) +{ +#ifdef CONFIG_EVENTFD + int fd = eventfd(!!active, EFD_NONBLOCK | EFD_CLOEXEC); + if (fd < 0) + return -errno; + e->fd = fd; + return 0; +#else + return -ENOSYS; +#endif +} + +void event_notifier_cleanup(EventNotifier *e) +{ + close(e->fd); +} + +int event_notifier_get_fd(EventNotifier *e) +{ + return e->fd; +} + +int event_notifier_test_and_clear(EventNotifier *e) +{ + uint64_t value; + int r = read(e->fd, &value, sizeof(value)); + return r == sizeof(value); +} + +int event_notifier_test(EventNotifier *e) +{ + uint64_t value; + int r = read(e->fd, &value, sizeof(value)); + if (r == sizeof(value)) { + /* restore previous value. */ + int s = write(e->fd, &value, sizeof(value)); + /* never blocks because we use EFD_SEMAPHORE. +* If we didn't we'd get EAGAIN on overflow +* and we'd have to write code to ignore it. */ + assert(s == sizeof(value)); + } + return r == sizeof(value); +} diff --git a/hw/notifier.h b/hw/notifier.h new file mode 100644 index 000..24117ea --- /dev/null +++ b/hw/notifier.h @@ -0,0 +1,16 @@ +#ifndef QEMU_EVENT_NOTIFIER_H +#define QEMU_EVENT_NOTIFIER_H + +#include "qemu-common.h" + +struct EventNotifier { + int fd; +}; + +int event_notifier_init(EventNotifier *, int active); +void event_notifier_cleanup(EventNotifier *); +int event_notifier_get_fd(EventNotifier *); +int event_notifier_test_and_clear(EventNotifier *); +int event_notifier_test(EventNotifier *); + +#endif diff --git a/qemu-common.h b/qemu-common.h index 805be1a..f12a8f5 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -227,6 +227,7 @@ typedef struct uWireSlave uWireSlave; typedef struct I2SCodec I2SCodec; typedef struct DeviceState DeviceState; typedef struct SSIBus SSIBus; +typedef struct EventNotifier EventNotifier; typedef uint64_t pcibus_t; -- 1.7.0.18.g0d53a5
[Qemu-devel] [PATCHv4 05/12] virtio: add APIs for queue fields
vhost needs physical addresses for ring and other queue fields, so add APIs for these. Signed-off-by: Michael S. Tsirkin --- hw/virtio.c | 76 +++ hw/virtio.h | 15 +++- 2 files changed, 90 insertions(+), 1 deletions(-) diff --git a/hw/virtio.c b/hw/virtio.c index 1f5e7be..e5787fa 100644 --- a/hw/virtio.c +++ b/hw/virtio.c @@ -74,6 +74,8 @@ struct VirtQueue uint16_t vector; void (*handle_output)(VirtIODevice *vdev, VirtQueue *vq); VirtIODevice *vdev; +EventNotifier guest_notifier; +EventNotifier host_notifier; }; /* virt queue functions */ @@ -593,6 +595,12 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size, return &vdev->vq[i]; } +void virtio_irq(VirtQueue *vq) +{ +vq->vdev->isr |= 0x01; +virtio_notify_vector(vq->vdev, vq->vector); +} + void virtio_notify(VirtIODevice *vdev, VirtQueue *vq) { /* Always notify when queue is empty (when feature acknowledge) */ @@ -736,3 +744,71 @@ void virtio_bind_device(VirtIODevice *vdev, const VirtIOBindings *binding, vdev->binding = binding; vdev->binding_opaque = opaque; } + +target_phys_addr_t virtio_queue_get_desc(VirtIODevice *vdev, int n) +{ +return vdev->vq[n].vring.desc; +} + +target_phys_addr_t virtio_queue_get_avail(VirtIODevice *vdev, int n) +{ +return vdev->vq[n].vring.avail; +} + +target_phys_addr_t virtio_queue_get_used(VirtIODevice *vdev, int n) +{ +return vdev->vq[n].vring.used; +} + +target_phys_addr_t virtio_queue_get_ring(VirtIODevice *vdev, int n) +{ +return vdev->vq[n].vring.desc; +} + +target_phys_addr_t virtio_queue_get_desc_size(VirtIODevice *vdev, int n) +{ +return sizeof(VRingDesc) * vdev->vq[n].vring.num; +} + +target_phys_addr_t virtio_queue_get_avail_size(VirtIODevice *vdev, int n) +{ +return offsetof(VRingAvail, ring) + +sizeof(u_int64_t) * vdev->vq[n].vring.num; +} + +target_phys_addr_t virtio_queue_get_used_size(VirtIODevice *vdev, int n) +{ +return offsetof(VRingUsed, ring) + +sizeof(VRingUsedElem) * vdev->vq[n].vring.num; +} + + +target_phys_addr_t virtio_queue_get_ring_size(VirtIODevice *vdev, int n) +{ +return vdev->vq[n].vring.used - vdev->vq[n].vring.desc + + virtio_queue_get_used_size(vdev, n); +} + +uint16_t virtio_queue_last_avail_idx(VirtIODevice *vdev, int n) +{ +return vdev->vq[n].last_avail_idx; +} + +void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx) +{ +vdev->vq[n].last_avail_idx = idx; +} + +VirtQueue *virtio_queue(VirtIODevice *vdev, int n) +{ +return vdev->vq + n; +} + +EventNotifier *virtio_queue_guest_notifier(VirtQueue *vq) +{ +return &vq->guest_notifier; +} +EventNotifier *virtio_queue_host_notifier(VirtQueue *vq) +{ +return &vq->host_notifier; +} diff --git a/hw/virtio.h b/hw/virtio.h index af87889..26cf5fd 100644 --- a/hw/virtio.h +++ b/hw/virtio.h @@ -184,5 +184,18 @@ void virtio_net_exit(VirtIODevice *vdev); DEFINE_PROP_BIT("indirect_desc", _state, _field, \ VIRTIO_RING_F_INDIRECT_DESC, true) - +target_phys_addr_t virtio_queue_get_desc(VirtIODevice *vdev, int n); +target_phys_addr_t virtio_queue_get_avail(VirtIODevice *vdev, int n); +target_phys_addr_t virtio_queue_get_used(VirtIODevice *vdev, int n); +target_phys_addr_t virtio_queue_get_ring(VirtIODevice *vdev, int n); +target_phys_addr_t virtio_queue_get_desc_size(VirtIODevice *vdev, int n); +target_phys_addr_t virtio_queue_get_avail_size(VirtIODevice *vdev, int n); +target_phys_addr_t virtio_queue_get_used_size(VirtIODevice *vdev, int n); +target_phys_addr_t virtio_queue_get_ring_size(VirtIODevice *vdev, int n); +uint16_t virtio_queue_last_avail_idx(VirtIODevice *vdev, int n); +void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx); +VirtQueue *virtio_queue(VirtIODevice *vdev, int n); +EventNotifier *virtio_queue_guest_notifier(VirtQueue *vq); +EventNotifier *virtio_queue_host_notifier(VirtQueue *vq); +void virtio_irq(VirtQueue *vq); #endif -- 1.7.0.18.g0d53a5
[Qemu-devel] [PATCHv4 04/12] virtio: add notifier support
Add binding API to set host/guest notifiers. Will be used by vhost. Signed-off-by: Michael S. Tsirkin --- hw/virtio.c |5 - hw/virtio.h |3 +++ 2 files changed, 7 insertions(+), 1 deletions(-) diff --git a/hw/virtio.c b/hw/virtio.c index 7c020a3..1f5e7be 100644 --- a/hw/virtio.c +++ b/hw/virtio.c @@ -73,6 +73,7 @@ struct VirtQueue int inuse; uint16_t vector; void (*handle_output)(VirtIODevice *vdev, VirtQueue *vq); +VirtIODevice *vdev; }; /* virt queue functions */ @@ -714,8 +715,10 @@ VirtIODevice *virtio_common_init(const char *name, uint16_t device_id, vdev->queue_sel = 0; vdev->config_vector = VIRTIO_NO_VECTOR; vdev->vq = qemu_mallocz(sizeof(VirtQueue) * VIRTIO_PCI_QUEUE_MAX); -for(i = 0; i < VIRTIO_PCI_QUEUE_MAX; i++) +for(i = 0; i < VIRTIO_PCI_QUEUE_MAX; i++) { vdev->vq[i].vector = VIRTIO_NO_VECTOR; +vdev->vq[i].vdev = vdev; +} vdev->name = name; vdev->config_len = config_size; diff --git a/hw/virtio.h b/hw/virtio.h index 3baa2a3..af87889 100644 --- a/hw/virtio.h +++ b/hw/virtio.h @@ -19,6 +19,7 @@ #include "qdev.h" #include "sysemu.h" #include "block_int.h" +#include "notifier.h" /* from Linux's linux/virtio_config.h */ @@ -89,6 +90,8 @@ typedef struct { int (*load_config)(void * opaque, QEMUFile *f); int (*load_queue)(void * opaque, int n, QEMUFile *f); unsigned (*get_features)(void * opaque); +int (*guest_notifier)(void * opaque, int n, bool assigned); +int (*host_notifier)(void * opaque, int n, bool assigned); } VirtIOBindings; #define VIRTIO_PCI_QUEUE_MAX 64 -- 1.7.0.18.g0d53a5
[Qemu-devel] [PATCHv4 02/12] kvm: add API to set ioeventfd
Comment on kvm usage: rather than require users to do if (kvm_enabled()) and/or ifdefs, this patch adds an API that, internally, is defined to stub function on non-kvm build, and checks kvm_enabled for non-kvm run. While rest of qemu code still uses if (kvm_enabled()), I think this approach is cleaner, and we should convert rest of code to it long term. Signed-off-by: Michael S. Tsirkin --- kvm-all.c | 22 ++ kvm.h | 16 2 files changed, 38 insertions(+), 0 deletions(-) diff --git a/kvm-all.c b/kvm-all.c index 1a02076..f54af71 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -1138,3 +1138,25 @@ int kvm_set_signal_mask(CPUState *env, const sigset_t *sigset) return r; } + +#ifdef KVM_IOEVENTFD +int kvm_set_ioeventfd_pio_word(int fd, uint16_t addr, uint16_t val, bool assign) +{ +struct kvm_ioeventfd kick = { +.datamatch = val, +.addr = addr, +.len = 2, +.flags = KVM_IOEVENTFD_FLAG_DATAMATCH | KVM_IOEVENTFD_FLAG_PIO, +.fd = fd, +}; +int r; +if (!kvm_enabled()) +return -ENOSYS; +if (!assign) +kick.flags |= KVM_IOEVENTFD_FLAG_DEASSIGN; +r = kvm_vm_ioctl(kvm_state, KVM_IOEVENTFD, &kick); +if (r < 0) +return r; +return 0; +} +#endif diff --git a/kvm.h b/kvm.h index a74dfcb..45d087b 100644 --- a/kvm.h +++ b/kvm.h @@ -14,10 +14,16 @@ #ifndef QEMU_KVM_H #define QEMU_KVM_H +#include +#include #include "config.h" #include "qemu-queue.h" #ifdef CONFIG_KVM +#include +#endif + +#ifdef CONFIG_KVM extern int kvm_allowed; #define kvm_enabled() (kvm_allowed) @@ -135,4 +141,14 @@ static inline void cpu_synchronize_state(CPUState *env) } } +#if defined(KVM_IOEVENTFD) && defined(CONFIG_KVM) +int kvm_set_ioeventfd_pio_word(int fd, uint16_t adr, uint16_t val, bool assign); +#else +static inline +int kvm_set_ioeventfd_pio_word(int fd, uint16_t adr, uint16_t val, bool assign) +{ +return -ENOSYS; +} +#endif + #endif -- 1.7.0.18.g0d53a5
[Qemu-devel] [PATCHv4 01/12] tap: add interface to get device fd
Will be used by vhost to attach/detach to backend. Signed-off-by: Michael S. Tsirkin --- net/tap.c |7 +++ net/tap.h |2 ++ 2 files changed, 9 insertions(+), 0 deletions(-) diff --git a/net/tap.c b/net/tap.c index 7a7320c..fc59fd4 100644 --- a/net/tap.c +++ b/net/tap.c @@ -269,6 +269,13 @@ static void tap_poll(VLANClientState *nc, bool enable) tap_write_poll(s, enable); } +int tap_get_fd(VLANClientState *nc) +{ +TAPState *s = DO_UPCAST(TAPState, nc, nc); +assert(nc->info->type == NET_CLIENT_TYPE_TAP); +return s->fd; +} + /* fd support */ static NetClientInfo net_tap_info = { diff --git a/net/tap.h b/net/tap.h index 538a562..a244b28 100644 --- a/net/tap.h +++ b/net/tap.h @@ -48,4 +48,6 @@ int tap_probe_vnet_hdr(int fd); int tap_probe_has_ufo(int fd); void tap_fd_set_offload(int fd, int csum, int tso4, int tso6, int ecn, int ufo); +int tap_get_fd(VLANClientState *vc); + #endif /* QEMU_NET_TAP_H */ -- 1.7.0.18.g0d53a5
[Qemu-devel] [PATCHv4 00/12] vhost-net: upstream integration
Here's a patchset with vhost support for upstream qemu, rebased to latest bits, and with all comments I'm aware of addressed. Please consider for merging. Anthony, if you are still deliberating some issues, maybe the series can be merged partially? This will at least reduce the amount of noise from reposting the large patchset. Changes from v3: vhost: vhost net support: use typedef instead of struct name virtio: add set_status callback: fix up non-PCI bindings Changes from v2: Addressed style comments Detect mapping changes and abort Unmap ring on cleanup Changes from v1: Addressed style comments Migration fixes. Gracefully fail with non-tap backends. Michael S. Tsirkin (12): tap: add interface to get device fd kvm: add API to set ioeventfd notifier: event notifier implementation virtio: add notifier support virtio: add APIs for queue fields virtio: add set_status callback virtio: move typedef to qemu-common virtio-pci: fill in notifier support vhost: vhost net support tap: add vhost/vhostfd options tap: add API to retrieve vhost net header virtio-net: vhost net support Makefile.target |3 + configure| 36 +++ hw/notifier.c| 62 + hw/notifier.h| 16 ++ hw/s390-virtio-bus.c |2 +- hw/syborg_virtio.c |2 +- hw/vhost.c | 706 ++ hw/vhost.h | 48 hw/vhost_net.c | 198 ++ hw/vhost_net.h | 19 ++ hw/virtio-net.c | 71 +- hw/virtio-pci.c | 67 +- hw/virtio.c | 81 ++- hw/virtio.h | 28 ++- kvm-all.c| 22 ++ kvm.h| 16 ++ net.c|8 + net/tap.c| 43 +++ net/tap.h|5 + qemu-common.h|2 + qemu-options.hx |4 +- 21 files changed, 1429 insertions(+), 10 deletions(-) create mode 100644 hw/notifier.c create mode 100644 hw/notifier.h create mode 100644 hw/vhost.c create mode 100644 hw/vhost.h create mode 100644 hw/vhost_net.c create mode 100644 hw/vhost_net.h
Re: [Qemu-devel] Re: [PATCHv2 10/12] tap: add vhost/vhostfd options
On Wed, Mar 03, 2010 at 08:15:15AM -0600, Anthony Liguori wrote: > On 03/02/2010 04:41 PM, Paul Brook wrote: > >>The new function I'm proposing has the following semantics: > >> > >>- it always returns a persistent mapping > >>- it never bounces > >>- it will only fail if the mapping isn't ram > >So you're assuming that virtio rings are in ram that is not hot-pluggable > > As long as the device is active, yes. This would be true with bare > metal. Memory allocated for the virtio-pci ring is not reclaimable > and you have to be able to reclaim the entire area of ram covered by > a DIMM to hot unplug it. A user would have to unload the virtio-pci > module to release the memory before hot unplug would be an option. > > NB, almost nothing supports memory hot remove because it's very > difficult for an OS to actually do. > > > or > >remapable, > > Yes, it cannot be remapable. > > > and the whole region is contiguous? > > Yes, it has to be contiguous. > > >That sounds like it's likely to come back and bite you. The guest has no idea > >which areas of ram happen to be contiguous on the host. > > Practically speaking, with target-i386 anything that is contiguous > in guest physical memory is contiguous in the host address space > provided it's ram. > > These assumptions are important. I have a local branch (that I'll > send out soon) that implements a ram API and converted virtio to > make use of it. I'm seeing a ~50% increase in tx throughput. > > If you try to support discontiguous, remapable ram for the virtio > ring, then you have to do an l1_phys_map lookup for every single > ring variable access followed by a memory copy. This ends up > costing an awful lot practically speaking. Speed up the lookup instead? > > The changes should work equally well with syborg although I don't > think I can do meaningful performance testing there (I don't > actually have a syborg image to test). > > Regards, > > Anthony Liguori > > >Paul
[Qemu-devel] Re: [PATCH 10/10] block: print errno on error
Paolo Bonzini wrote: >>> diff --git a/qemu-img.c b/qemu-img.c >>> index 0c9f2d4..f6c40fb 100644 >>> --- a/qemu-img.c >>> +++ b/qemu-img.c >>> @@ -374,7 +374,7 @@ static int img_create(int argc, char **argv) >>> } else if (ret == -EFBIG) { >>> error("The image size is too large for file format '%s'", >>> fmt); >>> } else { >>> -error("Error while formatting"); >>> +error("Error (%s) while formatting for file format '%s'", >>> strerror(ret), fmt); >>> } >>> } >>> return 0; >>> @@ -687,7 +687,7 @@ static int img_convert(int argc, char **argv) >>> } else if (ret == -EFBIG) { >>> error("The image size is too large for file format '%s'", >>> out_fmt); >>> } else { >>> -error("Error while formatting '%s'", out_filename); >>> +error("Error (%s) while formatting file '%s'", strerror(ret), >>> out_filename); >>> } >>> } >>> >> >> I think it should be strerror(-ret) in both cases. oops, yes. > Yes; also, since you are at it, I think that respectively > > error("%s: error while creating %s image: %s", filename, fmt, > strerror(-ret); > > error(%s: error while converting to %s: %s", out_filename, fmt, > strerror(-ret); > > would be more consistent with usual error messages: > > $ cat fdsfds > cat: fdsfds: No such file or directory I agree. I just didn't want to change it too much. > Paolo
[Qemu-devel] Re: [PATCH v4 03/10] x86: Extend validity of cpu_is_bsp
On Mon, Mar 01, 2010 at 06:17:22PM +0100, Jan Kiszka wrote: > As we hard-wire the BSP to CPU 0 anyway and cpuid_apic_id equals > cpu_index, cpu_is_bsp can also be based on the latter directly. This > will help an early user of it: KVM while initializing mp_state. > > Signed-off-by: Jan Kiszka > --- > hw/pc.c |3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/hw/pc.c b/hw/pc.c > index b90a79e..58c32ea 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -767,7 +767,8 @@ static void pc_init_ne2k_isa(NICInfo *nd) > > int cpu_is_bsp(CPUState *env) > { > -return env->cpuid_apic_id == 0; > +/* We hard-wire the BSP to the first CPU. */ > +return env->cpu_index == 0; > } We should not assume that. The function was written like that specifically so the code around it will not rely on this assumption. Now you change that specifically to write code that will do incorrect assumptions. I don't see the logic here. > > CPUState *pc_new_cpu(const char *cpu_model) > -- > 1.6.0.2 -- Gleb.
[Qemu-devel] [PATCH] qemu-nbd: Fix wrong description in qemu-nbd.texi
-c option needs argument but it's missing now. This patch fixes it. Signed-off-by: Ryota Ozaki --- qemu-nbd.texi |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/qemu-nbd.texi b/qemu-nbd.texi index ef13b34..44996cc 100644 --- a/qemu-nbd.texi +++ b/qemu-nbd.texi @@ -30,8 +30,8 @@ Export Qemu disk image using NBD protocol. use snapshot file @item -n, --nocache disable host cache -...@item -c, --connect - connect FILE to NBD device DEV +...@item -c, --conne...@var{dev} + connect @var{filename} to NBD device @var{dev} @item -d, --disconnect disconnect the specified device @item -e, --shar...@var{num} -- 1.6.5.2
[Qemu-devel] [PATCH] Build usb-ohci for PCs
The OHCI emulation isn't obviously broken and there are people who want to use it. Let's build it by default so that it can be enabled via -device. Signed-off-by: Kevin Wolf --- default-configs/i386-softmmu.mak |2 ++ default-configs/x86_64-softmmu.mak |2 ++ 2 files changed, 4 insertions(+), 0 deletions(-) diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak index 15586a0..9e6d565 100644 --- a/default-configs/i386-softmmu.mak +++ b/default-configs/i386-softmmu.mak @@ -1 +1,3 @@ # Default configuration for i386-softmmu + +CONFIG_USB_OHCI=y diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak index ec98af2..e0f7fc0 100644 --- a/default-configs/x86_64-softmmu.mak +++ b/default-configs/x86_64-softmmu.mak @@ -1 +1,3 @@ # Default configuration for x86_64-softmmu + +CONFIG_USB_OHCI=y -- 1.6.6.1
Re: [Qemu-devel] Re: [PATCHv2 10/12] tap: add vhost/vhostfd options
> > That sounds like it's likely to come back and bite you. The guest has no > > idea which areas of ram happen to be contiguous on the host. > > Practically speaking, with target-i386 anything that is contiguous in > guest physical memory is contiguous in the host address space provided > it's ram. > > These assumptions are important. I have a local branch (that I'll send > out soon) that implements a ram API and converted virtio to make use of > it. I'm seeing a ~50% increase in tx throughput. IMO supporting discontiguous regions is a requirement. target-i386 might get away with contiguous memory, because it omits most of the board level details. For everything else I'd expect this to be a real problem. I'm not happy about the not-remapable assumption either. In my experience this is fairly common. In fact many real x86 machines have this capability (to workaround the 4G PCI hole). By my reading the ppc440_bamboo board fails both your assumptions. I imagine the KVM-PPC folks would be upset if you decided that virtio no longer worked on this board. This is all somewhat disappointing, given virtio is supposed to be a DMA based architecture, and not rely on shared memory semantics. Paul
[Qemu-devel] Re: [PATCH 10/10] block: print errno on error
diff --git a/qemu-img.c b/qemu-img.c index 0c9f2d4..f6c40fb 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -374,7 +374,7 @@ static int img_create(int argc, char **argv) } else if (ret == -EFBIG) { error("The image size is too large for file format '%s'", fmt); } else { -error("Error while formatting"); +error("Error (%s) while formatting for file format '%s'", strerror(ret), fmt); } } return 0; @@ -687,7 +687,7 @@ static int img_convert(int argc, char **argv) } else if (ret == -EFBIG) { error("The image size is too large for file format '%s'", out_fmt); } else { -error("Error while formatting '%s'", out_filename); +error("Error (%s) while formatting file '%s'", strerror(ret), out_filename); } } I think it should be strerror(-ret) in both cases. Yes; also, since you are at it, I think that respectively error("%s: error while creating %s image: %s", filename, fmt, strerror(-ret); error(%s: error while converting to %s: %s", out_filename, fmt, strerror(-ret); would be more consistent with usual error messages: $ cat fdsfds cat: fdsfds: No such file or directory Paolo
[Qemu-devel] [PATCH] qemu-kvm: avoid strlen of NULL pointer
If the user wants to create a chardev of type socket but forgets to give a host= option, qemu_opt_get returns NULL. This NULL pointer is then fed into strlen a few lines below without a check which results in a segfault. This fixes it. Signed-off-by: Jens Osterkamp --- qemu-sockets.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/qemu-sockets.c b/qemu-sockets.c index 23c3def..a191304 100644 --- a/qemu-sockets.c +++ b/qemu-sockets.c @@ -137,6 +137,9 @@ int inet_listen_opts(QemuOpts *opts, int port_offset) pstrcpy(port, sizeof(port), qemu_opt_get(opts, "port")); addr = qemu_opt_get(opts, "host"); +if (!addr) + return -1; + to = qemu_opt_get_number(opts, "to", 0); if (qemu_opt_get_bool(opts, "ipv4", 0)) ai.ai_family = PF_INET; -- 1.5.6.3 -- Best regards, Jens Osterkamp IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter GeschƤftsfĆ¼hrung: Dirk Wittkopp Sitz der Gesellschaft: Bƶblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
[Qemu-devel] Re: Another VNC crash, qemu-kvm-0.12.3
On 03/01/2010 12:14 PM, Chris Webb wrote: We've just seen another VNC related qemu-kvm crash, this time an arithmetic exception at vnc.c:1424 in the newly release qemu-kvm 0.12.3. [...] 1423 if (vs->absolute) { 1424 kbd_mouse_event(x * 0x7FFF / (ds_get_width(vs->ds) - 1), 1425 y * 0x7FFF / (ds_get_height(vs->ds) - 1), 1426 dz, buttons); 1427 } else if (vnc_has_feature(vs, VNC_FEATURE_POINTER_TYPE_CHANGE)) { 1428 x -= 0x7FFF; [...] and sure enough: (gdb) p vs->ds->surface->width $1 = 9 (gdb) p vs->ds->surface->height $2 = 1 What a 9x1 display surface is doing on this guest is a mystery to me, but you definitely can't divide by one less than its height! Can you reproduce this reliably? If so, what's the procedure? BTW, I'd suggest filing this at http://bugs.launchpad.net/qemu Regards, Anthony Liguori (gdb) p *vs $3 = {csock = 19, ds = 0x1c60fa0, dirty = {{4294967295, 4294967295, 4294967295, 4294967295, 4294967295}}, vd = 0x26a0110, need_update = 1, force_update = 0, features = 67, absolute = 1, last_x = -1, last_y = -1, vnc_encoding = 5, tight_quality = 9 '\t', tight_compression = 9 '\t', major = 3, minor = 8, challenge = "Ā¹{\177\226\200kĆjĆ©PƱĆAĀ¤o)", output = {capacity = 925115, offset = 0, buffer = 0x28ba4b0 ""}, input = {capacity = 5120, offset = 6, buffer = 0x28b90a0 "\005"}, write_pixels = 0x4bb9e0, send_hextile_tile = 0x4bcdf0, clientds = {flags = 0 '\0', width = 800, height = 600, linesize = 3200, data = 0x7fcd00ab6010 "", pf = { bits_per_pixel = 32 ' ', bytes_per_pixel = 4 '\004', depth = 24 '\030', rmask = 0, gmask = 0, bmask = 0, amask = 0, rshift = 16 '\020', gshift = 8 '\b', bshift = 0 '\0', ashift = 24 '\030', rmax = 255 'Ćæ', gmax = 255 'Ćæ', bmax = 255 'Ćæ', amax = 255 'Ćæ', rbits = 8 '\b', gbits = 8 '\b', bbits = 8 '\b', abits = 8 '\b'}}, audio_cap = 0x0, as = {freq = 44100, nchannels = 2, fmt = AUD_FMT_S16, endianness = 0}, read_handler = 0x4beac0, read_handler_expect = 6, modifiers_state = '\0', zlib = {capacity = 0, offset = 0, buffer = 0x0}, zlib_tmp = {capacity = 0, offset = 0, buffer = 0x0}, zlib_stream = {{next_in = 0x0, avail_in = 0, total_in = 0, next_out = 0x0, avail_out = 0, total_out = 0, msg = 0x0, state = 0x0, zalloc = 0, zfree = 0, opaque = 0x0, data_type = 0, adler = 0, reserved = 0}, {next_in = 0x0, avail_in = 0, total_in = 0, next_out = 0x0, avail_out = 0, total_out = 0, msg = 0x0, state = 0x0, zalloc = 0, zfree = 0, opaque = 0x0, data_type = 0, adler = 0, reserved = 0}, {next_in = 0x0, avail_in = 0, total_in = 0, next_out = 0x0, avail_out = 0, total_out = 0, msg = 0x0, state = 0x0, zalloc = 0, zfree = 0, opaque = 0x0, data_type = 0, adler = 0, reserved = 0}, {next_in = 0x0, avail_in = 0, total_in = 0, next_out = 0x0, avail_out = 0, total_out = 0, msg = 0x0, state = 0x0, zalloc = 0, zfree = 0, opaque = 0x0, data_type = 0, adler = 0, reserved = 0}}, next = 0x0} (gdb) p *vs->ds $4 = {surface = 0x1c81f40, opaque = 0x26a0110, gui_timer = 0x0, allocator = 0x8199d0, listeners = 0x1c95fa0, mouse_set = 0, cursor_define = 0, next = 0x0} (gdb) p *vs->ds->surface $5 = {flags = 2 '\002', width = 9, height = 1, linesize = 36, data = 0x7fcd00ab6010 "", pf = { bits_per_pixel = 32 ' ', bytes_per_pixel = 4 '\004', depth = 24 '\030', rmask = 16711680, gmask = 65280, bmask = 255, amask = 0, rshift = 16 '\020', gshift = 8 '\b', bshift = 0 '\0', ashift = 24 '\030', rmax = 255 'Ćæ', gmax = 255 'Ćæ', bmax = 255 'Ćæ', amax = 255 'Ćæ', rbits = 8 '\b', gbits = 8 '\b', bbits = 8 '\b', abits = 8 '\b'}} Cheers, Chris. -- 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: [PATCHv2 10/12] tap: add vhost/vhostfd options
On 03/02/2010 04:41 PM, Paul Brook wrote: The new function I'm proposing has the following semantics: - it always returns a persistent mapping - it never bounces - it will only fail if the mapping isn't ram So you're assuming that virtio rings are in ram that is not hot-pluggable As long as the device is active, yes. This would be true with bare metal. Memory allocated for the virtio-pci ring is not reclaimable and you have to be able to reclaim the entire area of ram covered by a DIMM to hot unplug it. A user would have to unload the virtio-pci module to release the memory before hot unplug would be an option. NB, almost nothing supports memory hot remove because it's very difficult for an OS to actually do. or remapable, Yes, it cannot be remapable. and the whole region is contiguous? Yes, it has to be contiguous. That sounds like it's likely to come back and bite you. The guest has no idea which areas of ram happen to be contiguous on the host. Practically speaking, with target-i386 anything that is contiguous in guest physical memory is contiguous in the host address space provided it's ram. These assumptions are important. I have a local branch (that I'll send out soon) that implements a ram API and converted virtio to make use of it. I'm seeing a ~50% increase in tx throughput. If you try to support discontiguous, remapable ram for the virtio ring, then you have to do an l1_phys_map lookup for every single ring variable access followed by a memory copy. This ends up costing an awful lot practically speaking. The changes should work equally well with syborg although I don't think I can do meaningful performance testing there (I don't actually have a syborg image to test). Regards, Anthony Liguori Paul
[Qemu-devel] Re: [PATCH 10/10] block: print errno on error
Am 03.03.2010 13:07, schrieb Juan Quintela: > Now that we changed all create calls to return errno, just print it. > > Signed-off-by: Juan Quintela > --- > qemu-img.c |4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/qemu-img.c b/qemu-img.c > index 0c9f2d4..f6c40fb 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -374,7 +374,7 @@ static int img_create(int argc, char **argv) > } else if (ret == -EFBIG) { > error("The image size is too large for file format '%s'", fmt); > } else { > -error("Error while formatting"); > +error("Error (%s) while formatting for file format '%s'", > strerror(ret), fmt); > } > } > return 0; > @@ -687,7 +687,7 @@ static int img_convert(int argc, char **argv) > } else if (ret == -EFBIG) { > error("The image size is too large for file format '%s'", > out_fmt); > } else { > -error("Error while formatting '%s'", out_filename); > +error("Error (%s) while formatting file '%s'", strerror(ret), > out_filename); > } > } > I think it should be strerror(-ret) in both cases. Kevin
[Qemu-devel] Re: [PATCH 06/10] vl: exit if we are not able to write into the pipe
Kevin Wolf wrote: > Am 03.03.2010 13:06, schrieb Juan Quintela: >> Signed-off-by: Juan Quintela >> --- >> vl.c |1 + >> 1 files changed, 1 insertions(+), 0 deletions(-) >> >> diff --git a/vl.c b/vl.c >> index db7a178..119c7e4 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -5745,6 +5745,7 @@ int main(int argc, char **argv, char **envp) >> uint8_t status = 1; >> if (write(fds[1], &status, 1) != 1) { >> perror("daemonize. Writing to pipe\n"); >> +exit(1); >> } >> } else >> #endif > > This is already the handling code for a fatal error, it's just trying to > tell the parent process that it went wrong. Completing the context: > > fprintf(stderr, "Could not acquire pid file: %s\n", > strerror(errno)); > exit(1); > } > > So there is already an exit(1), this patch doesn't change anything. > > Kevin Else without braces :( You are right. Would remove on respin. Later, Juan.
[Qemu-devel] Re: [PATCH 06/10] vl: exit if we are not able to write into the pipe
Am 03.03.2010 13:06, schrieb Juan Quintela: > Signed-off-by: Juan Quintela > --- > vl.c |1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/vl.c b/vl.c > index db7a178..119c7e4 100644 > --- a/vl.c > +++ b/vl.c > @@ -5745,6 +5745,7 @@ int main(int argc, char **argv, char **envp) > uint8_t status = 1; > if (write(fds[1], &status, 1) != 1) { > perror("daemonize. Writing to pipe\n"); > +exit(1); > } > } else > #endif This is already the handling code for a fatal error, it's just trying to tell the parent process that it went wrong. Completing the context: fprintf(stderr, "Could not acquire pid file: %s\n", strerror(errno)); exit(1); } So there is already an exit(1), this patch doesn't change anything. Kevin
[Qemu-devel] Re: [PATCH 05/20] eepro100: Add all supported devices to pci.c
On Wed, Mar 03, 2010 at 02:31:53PM +0100, Gerd Hoffmann wrote: > On 03/03/10 12:51, Michael S. Tsirkin wrote: >> On Sun, Feb 21, 2010 at 10:08:49PM +0100, Stefan Weil wrote: > static const char * const pci_nic_models[] = { > "ne2k_pci", > +"i82550", > "i82551", > +"i82557a", > "i82557b", > +"i82557c", > +"i82558a", > +"i82558b", > +"i82559a", > +"i82559b", > +"i82559c", > "i82559er", > +"i82562", > "rtl8139", > "e1000", > "pcnet", > One wonders: would it be cleaner to have a single eepro100 device with specific model as qdev option? > > No. I think it would be cleaner to use qdev even more. For that > specific driver it would make sense to create a private Info struct, > i.e. something like this: > > E100PCIDeviceInfo { >PCIDeviceInfo pci; >[ model specific stuff such as has_extended_tcb_support here ] > }; > > Then go > > static E100PCIDeviceInfo eepro100_info[] = { > { > .pci.qdev.name = "i82550", > [ usual qdev stuff here ] > has_extended_tcb_support = 0 | 1; > [ more e100 device description bits here ] > }, > [ ... ] > }; > > Then you can simply zap all the one-liner wrapper functions around > nic_init(). nic_init() can get a pointer using something like this: > > E100PCIDeviceInfo *einfo = DO_UPCAST(E100PCIDeviceInfo, > pci.qdev, pci_dev->qdev.info); > > and setup the device accordingly. > >>> I prefer the "official" device names which are also >>> used in the Intel documentation. eepro100 or e100 >>> are marketing names (more of them exist). > > qdev has aliases. You can add .qdev.alias = "eepro100" to one of the > variants, then the eepro100 name will work as well. > > Adding the marketing name to the .desc test is probably a good idea too > because alot of people know the marketing name only. That's my thinking as well. >>> Please note that this patch was marked as optional. >>> The arrays pci_nic_names and pci_nic_models are >>> not really needed, and as soon as the table of available >>> nics is automatically derived from the device information, >>> all variants of i825xx are supported automatically. > >> Gerd, any input on this patch? > > Looks fine to me. When we switch over to walking the list of registered > devices instead of having a hard-coded list of pci nic drivers all the > eepro100 variants will show up in the list anyway. > > cheers, > Gerd -- MST
Re: [Qemu-devel] [PATCH 3/4] Fix signal handling for ColdFire
On 3/3/10 4:17 PM, Richard Henderson wrote: + float64 sc_fpregs[2]; /* room for two fp registers */ ... - int f_fpcntl[3]; - int f_fpregs[8*3]; + uint32_t f_fpcntl[3]; + float64 f_fpregs[8]; Surely these float64 uses are incorrect. The kernel uses 3*int at both of these places, which matches up with the 96-bit values in the regular m68k fpu. ColdFire FPU registers are 64-bit wide, this is one of the difference between ColdFire and classic m68k. The kernel patch to sigcontext.h to support ColdFire was merged several days ago at http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=arch/m68k/include/asm/sigcontext.h;h=1320eaa4cc2aab4b531a565c57ab62afb30bd0ec;hb=HEAD . And thanks to your comment I noticed that an old version of the sigcontext patch was merged into the kernel in the above commit. Will need to fix it. Regards, -- Maxim Kuvyrkov CodeSourcery ma...@codesourcery.com (650) 331-3385 x724
[Qemu-devel] Re: [PATCH 05/20] eepro100: Add all supported devices to pci.c
On 03/03/10 12:51, Michael S. Tsirkin wrote: On Sun, Feb 21, 2010 at 10:08:49PM +0100, Stefan Weil wrote: static const char * const pci_nic_models[] = { "ne2k_pci", +"i82550", "i82551", +"i82557a", "i82557b", +"i82557c", +"i82558a", +"i82558b", +"i82559a", +"i82559b", +"i82559c", "i82559er", +"i82562", "rtl8139", "e1000", "pcnet", One wonders: would it be cleaner to have a single eepro100 device with specific model as qdev option? No. I think it would be cleaner to use qdev even more. For that specific driver it would make sense to create a private Info struct, i.e. something like this: E100PCIDeviceInfo { PCIDeviceInfo pci; [ model specific stuff such as has_extended_tcb_support here ] }; Then go static E100PCIDeviceInfo eepro100_info[] = { { .pci.qdev.name = "i82550", [ usual qdev stuff here ] has_extended_tcb_support = 0 | 1; [ more e100 device description bits here ] }, [ ... ] }; Then you can simply zap all the one-liner wrapper functions around nic_init(). nic_init() can get a pointer using something like this: E100PCIDeviceInfo *einfo = DO_UPCAST(E100PCIDeviceInfo, pci.qdev, pci_dev->qdev.info); and setup the device accordingly. I prefer the "official" device names which are also used in the Intel documentation. eepro100 or e100 are marketing names (more of them exist). qdev has aliases. You can add .qdev.alias = "eepro100" to one of the variants, then the eepro100 name will work as well. Adding the marketing name to the .desc test is probably a good idea too because alot of people know the marketing name only. Please note that this patch was marked as optional. The arrays pci_nic_names and pci_nic_models are not really needed, and as soon as the table of available nics is automatically derived from the device information, all variants of i825xx are supported automatically. Gerd, any input on this patch? Looks fine to me. When we switch over to walking the list of registered devices instead of having a hard-coded list of pci nic drivers all the eepro100 variants will show up in the list anyway. cheers, Gerd
Re: [Qemu-devel] [PATCH 3/4] Fix signal handling for ColdFire
+float64sc_fpregs[2]; /* room for two fp registers */ ... -int f_fpcntl[3]; -int f_fpregs[8*3]; +uint32_t f_fpcntl[3]; +float64f_fpregs[8]; Surely these float64 uses are incorrect. The kernel uses 3*int at both of these places, which matches up with the 96-bit values in the regular m68k fpu. r~
[Qemu-devel] [PULL] eepro100
The following changes since commit 55b1e61f640bb2cf3bed0b4cc6d4ba1326c625d9: Samuel Thibault (1): (curses) Use more descriptive values are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mst/qemu.git for_anthony Stefan Weil (19): eepro100: Fix compiler errors from debug messages eepro100: Add missing SCB register names eepro100: Fix PXE boot eepro100: Support gpxe boot for all eepro100 devices eepro100: Add TODO list eepro100: Update copyright notice eepro100: Add device descriptions eepro100: Use symbolic names and BIT macros in binary operations eepro100: Remove old unused code eepro100: Use symbolic names for bits in EEPROM id eepro100: Replace variable name to fix a compiler warning eepro100: Support RNR interrupt eepro100: Fix CU Start command eepro100: Prettify code (no functional changes) eepro100: Use tx.status eepro100: New function for reading command block eepro100: Add diagnose command eepro100: Remove C++ comments eepro100: Keep includes sorted hw/eepro100.c | 436 +--- pc-bios/README |4 +- ...pxe-i82559er.bin => gpxe-eepro100-80861209.rom} | Bin 56832 -> 56832 bytes ...pxe-i82559er.bin => gpxe-eepro100-80861229.rom} | Bin 56832 -> 56832 bytes 4 files changed, 294 insertions(+), 146 deletions(-) copy pc-bios/{pxe-i82559er.bin => gpxe-eepro100-80861209.rom} (100%) rename pc-bios/{pxe-i82559er.bin => gpxe-eepro100-80861229.rom} (99%)
[Qemu-devel] [PATCH 10/10] block: print errno on error
Now that we changed all create calls to return errno, just print it. Signed-off-by: Juan Quintela --- qemu-img.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index 0c9f2d4..f6c40fb 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -374,7 +374,7 @@ static int img_create(int argc, char **argv) } else if (ret == -EFBIG) { error("The image size is too large for file format '%s'", fmt); } else { -error("Error while formatting"); +error("Error (%s) while formatting for file format '%s'", strerror(ret), fmt); } } return 0; @@ -687,7 +687,7 @@ static int img_convert(int argc, char **argv) } else if (ret == -EFBIG) { error("The image size is too large for file format '%s'", out_fmt); } else { -error("Error while formatting '%s'", out_filename); +error("Error (%s) while formatting file '%s'", strerror(ret), out_filename); } } -- 1.6.5.2
[Qemu-devel] [PATCH 09/10] vmdk: share cleanup code
cleanup code is identical for error/success cases. Only difference are goto labels. Signed-off-by: Juan Quintela --- block/vmdk.c | 13 - 1 files changed, 4 insertions(+), 9 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 819c1c9..007fca4 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -334,18 +334,13 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file) ret = -errno; goto fail_gd; } -qemu_free(gd_buf); -qemu_free(rgd_buf); - -close(p_fd); -close(snp_fd); -return 0; +ret = 0; -fail_gd: +fail_gd: qemu_free(gd_buf); -fail_rgd: +fail_rgd: qemu_free(rgd_buf); -fail: +fail: close(p_fd); close(snp_fd); return ret; -- 1.6.5.2
[Qemu-devel] [PATCH 08/10] vmdk: fix double free
fail_gd error case would also free rgd_buf that was already freed Signed-off-by: Juan Quintela --- block/vmdk.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 67a690e..819c1c9 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -315,7 +315,6 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file) ret = -errno; goto fail_rgd; } -qemu_free(rgd_buf); /* write GD */ gd_buf = qemu_malloc(gd_size); @@ -336,6 +335,7 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file) goto fail_gd; } qemu_free(gd_buf); +qemu_free(rgd_buf); close(p_fd); close(snp_fd); -- 1.6.5.2
[Qemu-devel] [PATCH 07/10] vmdk: make vmdk_snapshot_create return -errno
Signed-off-by: Juan Quintela --- block/vmdk.c | 79 ++--- 1 files changed, 58 insertions(+), 21 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 5b1d197..67a690e 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -187,6 +187,7 @@ static int vmdk_is_cid_valid(BlockDriverState *bs) static int vmdk_snapshot_create(const char *filename, const char *backing_file) { int snp_fd, p_fd; +int ret; uint32_t p_cid; char *p_name, *gd_buf, *rgd_buf; const char *real_filename, *temp_str; @@ -211,35 +212,49 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file) snp_fd = open(filename, O_RDWR | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE, 0644); if (snp_fd < 0) -return -1; +return -errno; p_fd = open(backing_file, O_RDONLY | O_BINARY | O_LARGEFILE); if (p_fd < 0) { close(snp_fd); -return -1; +return -errno; } /* read the header */ -if (lseek(p_fd, 0x0, SEEK_SET) == -1) +if (lseek(p_fd, 0x0, SEEK_SET) == -1) { +ret = -errno; goto fail; -if (read(p_fd, hdr, HEADER_SIZE) != HEADER_SIZE) +} +if (read(p_fd, hdr, HEADER_SIZE) != HEADER_SIZE) { +ret = -errno; goto fail; +} /* write the header */ -if (lseek(snp_fd, 0x0, SEEK_SET) == -1) +if (lseek(snp_fd, 0x0, SEEK_SET) == -1) { +ret = -errno; goto fail; -if (write(snp_fd, hdr, HEADER_SIZE) == -1) +} +if (write(snp_fd, hdr, HEADER_SIZE) == -1) { +ret = -errno; goto fail; +} memset(&header, 0, sizeof(header)); memcpy(&header,&hdr[4], sizeof(header)); // skip the VMDK4_MAGIC -if (ftruncate(snp_fd, header.grain_offset << 9)) +if (ftruncate(snp_fd, header.grain_offset << 9)) { +ret = -errno; goto fail; +} /* the descriptor offset = 0x200 */ -if (lseek(p_fd, 0x200, SEEK_SET) == -1) +if (lseek(p_fd, 0x200, SEEK_SET) == -1) { +ret = -errno; goto fail; -if (read(p_fd, p_desc, DESC_SIZE) != DESC_SIZE) +} +if (read(p_fd, p_desc, DESC_SIZE) != DESC_SIZE) { +ret = -errno; goto fail; +} if ((p_name = strstr(p_desc,"CID")) != NULL) { p_name += sizeof("CID"); @@ -258,10 +273,14 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file) (uint32_t)header.capacity, real_filename); /* write the descriptor */ -if (lseek(snp_fd, 0x200, SEEK_SET) == -1) +if (lseek(snp_fd, 0x200, SEEK_SET) == -1) { +ret = -errno; goto fail; -if (write(snp_fd, s_desc, strlen(s_desc)) == -1) +} +if (write(snp_fd, s_desc, strlen(s_desc)) == -1) { +ret = -errno; goto fail; +} gd_offset = header.gd_offset * SECTOR_SIZE; // offset of GD table rgd_offset = header.rgd_offset * SECTOR_SIZE; // offset of RGD table @@ -271,33 +290,51 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file) * 512 GTE per GT, each GTE points to grain */ gt_size = (int64_t)header.num_gtes_per_gte * header.granularity * SECTOR_SIZE; -if (!gt_size) +if (!gt_size) { +ret = -EINVAL; goto fail; +} gde_entries = (uint32_t)(capacity / gt_size); // number of gde/rgde gd_size = gde_entries * sizeof(uint32_t); /* write RGD */ rgd_buf = qemu_malloc(gd_size); -if (lseek(p_fd, rgd_offset, SEEK_SET) == -1) +if (lseek(p_fd, rgd_offset, SEEK_SET) == -1) { +ret = -errno; goto fail_rgd; -if (read(p_fd, rgd_buf, gd_size) != gd_size) +} +if (read(p_fd, rgd_buf, gd_size) != gd_size) { +ret = -errno; goto fail_rgd; -if (lseek(snp_fd, rgd_offset, SEEK_SET) == -1) +} +if (lseek(snp_fd, rgd_offset, SEEK_SET) == -1) { +ret = -errno; goto fail_rgd; -if (write(snp_fd, rgd_buf, gd_size) == -1) +} +if (write(snp_fd, rgd_buf, gd_size) == -1) { +ret = -errno; goto fail_rgd; +} qemu_free(rgd_buf); /* write GD */ gd_buf = qemu_malloc(gd_size); -if (lseek(p_fd, gd_offset, SEEK_SET) == -1) +if (lseek(p_fd, gd_offset, SEEK_SET) == -1) { +ret = -errno; goto fail_gd; -if (read(p_fd, gd_buf, gd_size) != gd_size) +} +if (read(p_fd, gd_buf, gd_size) != gd_size) { +ret = -errno; goto fail_gd; -if (lseek(snp_fd, gd_offset, SEEK_SET) == -1) +} +if (lseek(snp_fd, gd_offset, SEEK_SET) == -1) { +ret = -errno; goto fail_gd; -if (write(snp_fd, gd_buf, gd_size) == -1) +} +if (write(snp_fd, gd_buf, gd_size) == -1) { +ret = -errno; goto fail_gd; +} qemu_free(gd_buf); close(p_fd); @@ -311,7 +348,7 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file) fail: close(p_fd);
[Qemu-devel] [PATCH 06/10] vl: exit if we are not able to write into the pipe
Signed-off-by: Juan Quintela --- vl.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/vl.c b/vl.c index db7a178..119c7e4 100644 --- a/vl.c +++ b/vl.c @@ -5745,6 +5745,7 @@ int main(int argc, char **argv, char **envp) uint8_t status = 1; if (write(fds[1], &status, 1) != 1) { perror("daemonize. Writing to pipe\n"); +exit(1); } } else #endif -- 1.6.5.2
[Qemu-devel] [PATCH 05/10] vmdk: return errno instead of -1
Signed-off-by: Juan Quintela --- block/vmdk.c | 14 +++--- 1 files changed, 7 insertions(+), 7 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 56c28a0..5b1d197 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -740,7 +740,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE, 0644); if (fd < 0) -return -1; +return -errno; magic = cpu_to_be32(VMDK4_MAGIC); memset(&header, 0, sizeof(header)); header.version = cpu_to_le32(1); @@ -777,18 +777,18 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) /* write all the data */ ret = qemu_write_full(fd, &magic, sizeof(magic)); if (ret != sizeof(magic)) { -ret = -1; +ret = -errno; goto exit; } ret = qemu_write_full(fd, &header, sizeof(header)); if (ret != sizeof(header)) { -ret = -1; +ret = -errno; goto exit; } ret = ftruncate(fd, header.grain_offset << 9); if (ret < 0) { -ret = -1; +ret = -errno; goto exit; } @@ -798,7 +798,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) i < gt_count; i++, tmp += gt_size) { ret = qemu_write_full(fd, &tmp, sizeof(tmp)); if (ret != sizeof(tmp)) { -ret = -1; +ret = -errno; goto exit; } } @@ -809,7 +809,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) i < gt_count; i++, tmp += gt_size) { ret = qemu_write_full(fd, &tmp, sizeof(tmp)); if (ret != sizeof(tmp)) { -ret = -1; +ret = -errno; goto exit; } } @@ -831,7 +831,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options) lseek(fd, le64_to_cpu(header.desc_offset) << 9, SEEK_SET); ret = qemu_write_full(fd, desc, strlen(desc)); if (ret != strlen(desc)) { -ret = -1; +ret = -errno; goto exit; } -- 1.6.5.2
[Qemu-devel] [PATCH 04/10] qcow: return errno instead of -1
Signed-off-by: Juan Quintela --- block/qcow.c |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/block/qcow.c b/block/qcow.c index 003db1e..c619984 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -766,7 +766,7 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options) fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644); if (fd < 0) -return -1; +return -errno; memset(&header, 0, sizeof(header)); header.magic = cpu_to_be32(QCOW_MAGIC); header.version = cpu_to_be32(QCOW_VERSION); @@ -804,14 +804,14 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options) /* write all the data */ ret = qemu_write_full(fd, &header, sizeof(header)); if (ret != sizeof(header)) { -ret = -1; +ret = -errno; goto exit; } if (backing_file) { ret = qemu_write_full(fd, backing_file, backing_filename_len); if (ret != backing_filename_len) { -ret = -1; +ret = -errno; goto exit; } @@ -821,7 +821,7 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options) for(i = 0;i < l1_size; i++) { ret = qemu_write_full(fd, &tmp, sizeof(tmp)); if (ret != sizeof(tmp)) { -ret = -1; +ret = -errno; goto exit; } } -- 1.6.5.2
[Qemu-devel] [PATCH 03/10] qcow2: return errno instead of -1
Signed-off-by: Juan Quintela --- block/qcow2.c | 18 +- 1 files changed, 9 insertions(+), 9 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index bf8170e..5b6dad9 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -851,7 +851,7 @@ static int qcow_create2(const char *filename, int64_t total_size, fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644); if (fd < 0) -return -1; +return -errno; memset(&header, 0, sizeof(header)); header.magic = cpu_to_be32(QCOW_MAGIC); header.version = cpu_to_be32(QCOW_VERSION); @@ -930,7 +930,7 @@ static int qcow_create2(const char *filename, int64_t total_size, /* write all the data */ ret = qemu_write_full(fd, &header, sizeof(header)); if (ret != sizeof(header)) { -ret = -1; +ret = -errno; goto exit; } if (backing_file) { @@ -943,25 +943,25 @@ static int qcow_create2(const char *filename, int64_t total_size, cpu_to_be32s(&ext_bf.len); ret = qemu_write_full(fd, &ext_bf, sizeof(ext_bf)); if (ret != sizeof(ext_bf)) { -ret = -1; +ret = -errno; goto exit; } ret = qemu_write_full(fd, backing_format, backing_format_len); if (ret != backing_format_len) { -ret = -1; +ret = -errno; goto exit; } if (padding > 0) { ret = qemu_write_full(fd, zero, padding); if (ret != padding) { -ret = -1; +ret = -errno; goto exit; } } } ret = qemu_write_full(fd, backing_file, backing_filename_len); if (ret != backing_filename_len) { -ret = -1; +ret = -errno; goto exit; } } @@ -970,14 +970,14 @@ static int qcow_create2(const char *filename, int64_t total_size, for(i = 0;i < l1_size; i++) { ret = qemu_write_full(fd, &tmp, sizeof(tmp)); if (ret != sizeof(tmp)) { -ret = -1; +ret = -errno; goto exit; } } lseek(fd, s->refcount_table_offset, SEEK_SET); ret = qemu_write_full(fd, s->refcount_table, s->cluster_size); if (ret != s->cluster_size) { -ret = -1; +ret = -errno; goto exit; } @@ -985,7 +985,7 @@ static int qcow_create2(const char *filename, int64_t total_size, ret = qemu_write_full(fd, s->refcount_block, ref_clusters * s->cluster_size); if (ret != ref_clusters * s->cluster_size) { -ret = -1; +ret = -errno; goto exit; } -- 1.6.5.2
[Qemu-devel] [PATCH 02/10] slirp: check system() success
we shouldn't call W*() macros until we check that fork worked. Signed-off-by: Juan Quintela --- net/slirp.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/net/slirp.c b/net/slirp.c index 317cca7..7f846ec 100644 --- a/net/slirp.c +++ b/net/slirp.c @@ -472,7 +472,7 @@ static void slirp_smb_cleanup(SlirpState *s) if (s->smb_dir[0] != '\0') { snprintf(cmd, sizeof(cmd), "rm -rf %s", s->smb_dir); ret = system(cmd); -if (!WIFEXITED(ret)) { +if (ret == -1 || !WIFEXITED(ret)) { qemu_error("'%s' failed.\n", cmd); } else if (WEXITSTATUS(ret)) { qemu_error("'%s' failed. Error code: %d\n", -- 1.6.5.2
[Qemu-devel] [PATCH 01/10] cow: return errno instead of -1
Remove not needed ret = 0 assignment. Signed-off-by: Juan Quintela --- block/cow.c |5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/block/cow.c b/block/cow.c index 3733385..97e9745 100644 --- a/block/cow.c +++ b/block/cow.c @@ -224,7 +224,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options) cow_fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644); if (cow_fd < 0) -return -1; +return -errno; memset(&cow_header, 0, sizeof(cow_header)); cow_header.magic = cpu_to_be32(COW_MAGIC); cow_header.version = cpu_to_be32(COW_VERSION); @@ -251,7 +251,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options) cow_header.size = cpu_to_be64(image_sectors * 512); ret = qemu_write_full(cow_fd, &cow_header, sizeof(cow_header)); if (ret != sizeof(cow_header)) { -ret = -1; +ret = -errno; goto exit; } @@ -262,7 +262,6 @@ static int cow_create(const char *filename, QEMUOptionParameter *options) goto exit; } -ret = 0; exit: close(cow_fd); return ret; -- 1.6.5.2
[Qemu-devel] [PATCH 00/10] FORTIFY_SOURCE followup
Hi This series make: - all block *_create() functions return -errno instead of -1 - this makes that we can end writting errno/error at bdrv_create() callers (qemu-img) - once there found a double free problem in the error handling of vmdk, fixed it. - slirp: also check that system() was able to fork (amit noticed it) - daemonize: if we are unable to write into the pipe, print a message and exit. We can't really recover from that error (amit noticed it). Please review and apply. Later, Juan. Juan Quintela (10): cow: return errno instead of -1 slirp: check system() success qcow2: return errno instead of -1 qcow: return errno instead of -1 vmdk: return errno instead of -1 vl: exit if we are not able to write into the pipe vmdk: make vmdk_snapshot_create return -errno vmdk: fix double free vmdk: share cleanup code block: print errno on error block/cow.c |5 +-- block/qcow.c |8 ++-- block/qcow2.c | 18 +- block/vmdk.c | 106 + net/slirp.c |2 +- qemu-img.c|4 +- vl.c |1 + 7 files changed, 88 insertions(+), 56 deletions(-)
[Qemu-devel] [PATCH] eepro100: address pci todo's, use pci_set_xx
eepro100 uses macros which rely on a specific local variable name (pci_conf) which is scary. Some of the uses are wrong or unnecessary, remove them. The rest are small in number, open-code them using pci_set_xx functions. Signed-off-by: Michael S. Tsirkin --- The following is untested. Pls take a look, if you ack this I'll put this on my tree as well. hw/eepro100.c | 94 +++- 1 files changed, 32 insertions(+), 62 deletions(-) diff --git a/hw/eepro100.c b/hw/eepro100.c index 45ab497..7db6fb5 100644 --- a/hw/eepro100.c +++ b/hw/eepro100.c @@ -48,15 +48,6 @@ #include "net.h" #include "eeprom93xx.h" -/* Common declarations for all PCI devices. */ - -#define PCI_CONFIG_8(offset, value) \ -(pci_conf[offset] = (value)) -#define PCI_CONFIG_16(offset, value) \ -(*(uint16_t *)&pci_conf[offset] = cpu_to_le16(value)) -#define PCI_CONFIG_32(offset, value) \ -(*(uint32_t *)&pci_conf[offset] = cpu_to_le32(value)) - #define KiB 1024 /* Debug EEPRO100 card. */ @@ -467,49 +458,28 @@ static void pci_reset(EEPRO100State * s) /* PCI Vendor ID */ pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL); /* PCI Device ID depends on device and is set below. */ -/* PCI Command */ -/* TODO: this is the default, do not override. */ -PCI_CONFIG_16(PCI_COMMAND, 0x); /* PCI Status */ -/* TODO: Value at RST# should be 0. */ -PCI_CONFIG_16(PCI_STATUS, PCI_STATUS_DEVSEL_MEDIUM | PCI_STATUS_FAST_BACK); +pci_set_word(pci_conf + PCI_STATUS, PCI_STATUS_DEVSEL_MEDIUM | PCI_STATUS_FAST_BACK); /* PCI Revision ID */ -PCI_CONFIG_8(PCI_REVISION_ID, 0x08); -/* TODO: this is the default, do not override. */ -/* PCI Class Code */ -PCI_CONFIG_8(PCI_CLASS_PROG, 0x00); +pci_set_byte(pci_conf + PCI_REVISION_ID, 0x08); pci_config_set_class(pci_conf, PCI_CLASS_NETWORK_ETHERNET); -/* PCI Cache Line Size */ -/* check cache line size!!! */ -#if 0 -PCI_CONFIG_8(0x0c, 0x00); -#endif /* PCI Latency Timer */ -PCI_CONFIG_8(PCI_LATENCY_TIMER, 0x20); /* latency timer = 32 clocks */ -/* PCI Header Type */ -/* BIST (built-in self test) */ -/* Expansion ROM Base Address (depends on boot disable!!!) */ -/* TODO: not needed, set when BAR is registered */ -PCI_CONFIG_32(PCI_ROM_ADDRESS, PCI_BASE_ADDRESS_SPACE_MEMORY); +pci_set_byte(pci_conf + PCI_LATENCY_TIMER, 0x20); /* latency timer = 32 clocks */ /* Capability Pointer */ /* TODO: revisions with power_management 1 use this but * do not set new capability list bit in status register. */ -PCI_CONFIG_8(PCI_CAPABILITY_LIST, 0xdc); -/* Interrupt Line */ -/* Interrupt Pin */ -/* TODO: RST# value should be 0 */ -PCI_CONFIG_8(PCI_INTERRUPT_PIN, 1); /* interrupt pin 0 */ +pci_set_byte(pci_conf + PCI_CAPABILITY_LIST, 0xdc); /* Minimum Grant */ -PCI_CONFIG_8(PCI_MIN_GNT, 0x08); +pci_set_byte(pci_conf + PCI_MIN_GNT, 0x08); /* Maximum Latency */ -PCI_CONFIG_8(PCI_MAX_LAT, 0x18); +pci_set_byte(pci_conf + PCI_MAX_LAT, 0x18); switch (device) { case i82550: /* TODO: check device id. */ pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82551IT); /* Revision ID: 0x0c, 0x0d, 0x0e. */ -PCI_CONFIG_8(PCI_REVISION_ID, 0x0e); +pci_set_byte(pci_conf + PCI_REVISION_ID, 0x0e); /* TODO: check size of statistical counters. */ s->stats_size = 80; /* TODO: check extended tcb support. */ @@ -518,80 +488,80 @@ static void pci_reset(EEPRO100State * s) case i82551: pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82551IT); /* Revision ID: 0x0f, 0x10. */ -PCI_CONFIG_8(PCI_REVISION_ID, 0x0f); +pci_set_byte(pci_conf + PCI_REVISION_ID, 0x0f); /* TODO: check size of statistical counters. */ s->stats_size = 80; s->has_extended_tcb_support = 1; break; case i82557A: pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82557); -PCI_CONFIG_8(PCI_REVISION_ID, 0x01); -PCI_CONFIG_8(PCI_CAPABILITY_LIST, 0x00); +pci_set_byte(pci_conf + PCI_REVISION_ID, 0x01); +pci_set_byte(pci_conf + PCI_CAPABILITY_LIST, 0x00); power_management = 0; break; case i82557B: pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82557); -PCI_CONFIG_8(PCI_REVISION_ID, 0x02); -PCI_CONFIG_8(PCI_CAPABILITY_LIST, 0x00); +pci_set_byte(pci_conf + PCI_REVISION_ID, 0x02); +pci_set_byte(pci_conf + PCI_CAPABILITY_LIST, 0x00); power_management = 0; break; case i82557C: pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82557); -PCI_CONFIG_8(PCI_REVISION_ID, 0x03); -PCI_CONFIG_8(PCI_CAPABILITY_LIST, 0x00); +pci_set_byte(pci_conf + PCI_REVISION_ID, 0x03); +pci_set_byte(pci_conf + PCI_CAPABIL
[Qemu-devel] Re: [PATCHv3 01/20] eepro100: Fix compiler errors from debug messages
Applied the series except patch 5. Thanks! -- MST
[Qemu-devel] Re: [PATCH 05/20] eepro100: Add all supported devices to pci.c
On Sun, Feb 21, 2010 at 10:08:49PM +0100, Stefan Weil wrote: > Michael S. Tsirkin schrieb: > > On Sun, Feb 14, 2010 at 05:16:14PM +0100, Stefan Weil wrote: > > > >> All eepro100 devices work with drivers which > >> only use basic features. > >> > >> They were tested with gpxe boot. > >> > >> Signed-off-by: Stefan Weil > >> --- > >> hw/pci.c | 18 ++ > >> 1 files changed, 18 insertions(+), 0 deletions(-) > >> > >> diff --git a/hw/pci.c b/hw/pci.c > >> index eb2043e..1ba3f92 100644 > >> --- a/hw/pci.c > >> +++ b/hw/pci.c > >> @@ -1436,9 +1436,18 @@ void do_pci_info(Monitor *mon, QObject **ret_data) > >> > >> static const char * const pci_nic_models[] = { > >> "ne2k_pci", > >> +"i82550", > >> "i82551", > >> +"i82557a", > >> "i82557b", > >> +"i82557c", > >> +"i82558a", > >> +"i82558b", > >> +"i82559a", > >> +"i82559b", > >> +"i82559c", > >> "i82559er", > >> +"i82562", > >> "rtl8139", > >> "e1000", > >> "pcnet", > >> @@ -1448,9 +1457,18 @@ static const char * const pci_nic_models[] = { > >> > >> static const char * const pci_nic_names[] = { > >> "ne2k_pci", > >> +"i82550", > >> "i82551", > >> +"i82557a", > >> "i82557b", > >> +"i82557c", > >> +"i82558a", > >> +"i82558b", > >> +"i82559a", > >> +"i82559b", > >> +"i82559c", > >> "i82559er", > >> +"i82562", > >> "rtl8139", > >> "e1000", > >> "pcnet", > >> > > > > One wonders: would it be cleaner to have a single eepro100 device > > with specific model as qdev option? > > Technically that would be possible, too. > You could even have a single pci ethernet device and > specify vendor and device id as qdev options. > > I prefer the "official" device names which are also > used in the Intel documentation. eepro100 or e100 > are marketing names (more of them exist). > > Please note that this patch was marked as optional. > The arrays pci_nic_names and pci_nic_models are > not really needed, and as soon as the table of available > nics is automatically derived from the device information, > all variants of i825xx are supported automatically. > > Regards > Stefan Weil Gerd, any input on this patch? -- MST
[Qemu-devel] Re: [PATCHv3 08/20] eepro100: Add device descriptions
On Tue, Mar 02, 2010 at 10:37:48PM +0100, Stefan Weil wrote: > Add descriptions for all devices. > These descriptions are shown when users call > qemu -device ? > > Signed-off-by: Stefan Weil I still think descriptive names should also mention eepro100. > --- > hw/eepro100.c | 12 > 1 files changed, 12 insertions(+), 0 deletions(-) > > diff --git a/hw/eepro100.c b/hw/eepro100.c > index 6c11209..d5d3abc 100644 > --- a/hw/eepro100.c > +++ b/hw/eepro100.c > @@ -1980,6 +1980,7 @@ static int pci_i82562_init(PCIDevice *pci_dev) > static PCIDeviceInfo eepro100_info[] = { > { > .qdev.name = "i82550", > +.qdev.desc = "Intel i82550 Ethernet", > .qdev.size = sizeof(EEPRO100State), > .init = pci_i82550_init, > .exit = pci_nic_uninit, > @@ -1990,6 +1991,7 @@ static PCIDeviceInfo eepro100_info[] = { > }, > },{ > .qdev.name = "i82551", > +.qdev.desc = "Intel i82551 Ethernet", > .qdev.size = sizeof(EEPRO100State), > .init = pci_i82551_init, > .exit = pci_nic_uninit, > @@ -2000,6 +2002,7 @@ static PCIDeviceInfo eepro100_info[] = { > }, > },{ > .qdev.name = "i82557a", > +.qdev.desc = "Intel i82557A Ethernet", > .qdev.size = sizeof(EEPRO100State), > .init = pci_i82557a_init, > .exit = pci_nic_uninit, > @@ -2010,6 +2013,7 @@ static PCIDeviceInfo eepro100_info[] = { > }, > },{ > .qdev.name = "i82557b", > +.qdev.desc = "Intel i82557B Ethernet", > .qdev.size = sizeof(EEPRO100State), > .init = pci_i82557b_init, > .exit = pci_nic_uninit, > @@ -2020,6 +2024,7 @@ static PCIDeviceInfo eepro100_info[] = { > }, > },{ > .qdev.name = "i82557c", > +.qdev.desc = "Intel i82557C Ethernet", > .qdev.size = sizeof(EEPRO100State), > .init = pci_i82557c_init, > .exit = pci_nic_uninit, > @@ -2030,6 +2035,7 @@ static PCIDeviceInfo eepro100_info[] = { > }, > },{ > .qdev.name = "i82558a", > +.qdev.desc = "Intel i82558A Ethernet", > .qdev.size = sizeof(EEPRO100State), > .init = pci_i82558a_init, > .exit = pci_nic_uninit, > @@ -2040,6 +2046,7 @@ static PCIDeviceInfo eepro100_info[] = { > }, > },{ > .qdev.name = "i82558b", > +.qdev.desc = "Intel i82558B Ethernet", > .qdev.size = sizeof(EEPRO100State), > .init = pci_i82558b_init, > .exit = pci_nic_uninit, > @@ -2050,6 +2057,7 @@ static PCIDeviceInfo eepro100_info[] = { > }, > },{ > .qdev.name = "i82559a", > +.qdev.desc = "Intel i82559A Ethernet", > .qdev.size = sizeof(EEPRO100State), > .init = pci_i82559a_init, > .exit = pci_nic_uninit, > @@ -2060,6 +2068,7 @@ static PCIDeviceInfo eepro100_info[] = { > }, > },{ > .qdev.name = "i82559b", > +.qdev.desc = "Intel i82559B Ethernet", > .qdev.size = sizeof(EEPRO100State), > .init = pci_i82559b_init, > .exit = pci_nic_uninit, > @@ -2070,6 +2079,7 @@ static PCIDeviceInfo eepro100_info[] = { > }, > },{ > .qdev.name = "i82559c", > +.qdev.desc = "Intel i82559C Ethernet", > .qdev.size = sizeof(EEPRO100State), > .init = pci_i82559c_init, > .exit = pci_nic_uninit, > @@ -2080,6 +2090,7 @@ static PCIDeviceInfo eepro100_info[] = { > }, > },{ > .qdev.name = "i82559er", > +.qdev.desc = "Intel i82559ER Ethernet", > .qdev.size = sizeof(EEPRO100State), > .init = pci_i82559er_init, > .exit = pci_nic_uninit, > @@ -2090,6 +2101,7 @@ static PCIDeviceInfo eepro100_info[] = { > }, > },{ > .qdev.name = "i82562", > +.qdev.desc = "Intel i82562 Ethernet", > .qdev.size = sizeof(EEPRO100State), > .init = pci_i82562_init, > .exit = pci_nic_uninit, > -- > 1.7.0
[Qemu-devel] Re: [PATCHv3 19/20] eepro100: Remove C++ comments
On Tue, Mar 02, 2010 at 10:37:59PM +0100, Stefan Weil wrote: > C++ comments are unwanted, so this is fixed here. > > * Replace C++ comments by C comments. > * Put code which was deactivated by a C++ comment in #if 0...#endif. > > Signed-off-by: Stefan Weil It would be nice to add some documentation to commented blocks, explaining why we still keep them around and what happens if they are uncommented. > --- > hw/eepro100.c | 185 > +++-- > 1 files changed, 126 insertions(+), 59 deletions(-) > > diff --git a/hw/eepro100.c b/hw/eepro100.c > index 0f07b70..43a9060 100644 > --- a/hw/eepro100.c > +++ b/hw/eepro100.c > @@ -154,11 +154,13 @@ typedef struct { > uint16_t tcb_bytes; /* transmit command block byte count (in > lower 14 bits */ > uint8_t tx_threshold; /* transmit threshold */ > uint8_t tbd_count; /* TBD number */ > -//~ /* This constitutes two "TBD" entries: hdr and data */ > -//~ uint32_t tx_buf_addr0; /* void *, header of frame to be > transmitted. */ > -//~ int32_t tx_buf_size0; /* Length of Tx hdr. */ > -//~ uint32_t tx_buf_addr1; /* void *, data to be transmitted. */ > -//~ int32_t tx_buf_size1; /* Length of Tx data. */ > +#if 0 > +/* This constitutes two "TBD" entries: hdr and data */ > +uint32_t tx_buf_addr0; /* void *, header of frame to be transmitted. */ > +int32_t tx_buf_size0; /* Length of Tx hdr. */ > +uint32_t tx_buf_addr1; /* void *, data to be transmitted. */ > +int32_t tx_buf_size1; /* Length of Tx data. */ > +#endif Is it likely we'll uncomment this at some point? I note that you now use sizeof this structure: will adding these fields break anything? > } eepro100_tx_t; > > /* Receive frame descriptor. */ > @@ -398,7 +400,9 @@ static void eepro100_interrupt(EEPRO100State * s, uint8_t > status) > s->mem[SCBAck] |= status; > status = s->scb_stat = s->mem[SCBAck]; > status &= (mask | 0x0f); > -//~ status &= (~s->mem[SCBIntmask] | 0x0xf); > +#if 0 > +status &= (~s->mem[SCBIntmask] | 0x0xf); > +#endif > if (status && (mask & 0x01)) { > /* SCB mask and SCB Bit M do not disable interrupt. */ > enable_interrupt(s); > @@ -477,9 +481,11 @@ static void pci_reset(EEPRO100State * s) > pci_config_set_class(pci_conf, PCI_CLASS_NETWORK_ETHERNET); > /* PCI Cache Line Size */ > /* check cache line size!!! */ > -//~ PCI_CONFIG_8(0x0c, 0x00); > +#if 0 > +PCI_CONFIG_8(0x0c, 0x00); > +#endif It's safe to kill the above, 0 is the default anyway. > /* PCI Latency Timer */ > -PCI_CONFIG_8(PCI_LATENCY_TIMER, 0x20); // latency timer = 32 clocks > +PCI_CONFIG_8(PCI_LATENCY_TIMER, 0x20); /* latency timer = 32 clocks */ > /* PCI Header Type */ > /* BIST (built-in self test) */ > /* Expansion ROM Base Address (depends on boot disable!!!) */ > @@ -492,7 +498,7 @@ static void pci_reset(EEPRO100State * s) > /* Interrupt Line */ > /* Interrupt Pin */ > /* TODO: RST# value should be 0 */ > -PCI_CONFIG_8(PCI_INTERRUPT_PIN, 1); // interrupt pin 0 > +PCI_CONFIG_8(PCI_INTERRUPT_PIN, 1); /* interrupt pin 0 */ > /* Minimum Grant */ > PCI_CONFIG_8(PCI_MIN_GNT, 0x08); > /* Maximum Latency */ > @@ -500,20 +506,20 @@ static void pci_reset(EEPRO100State * s) > > switch (device) { > case i82550: > -// TODO: check device id. > +/* TODO: check device id. */ > pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82551IT); > /* Revision ID: 0x0c, 0x0d, 0x0e. */ > PCI_CONFIG_8(PCI_REVISION_ID, 0x0e); > -// TODO: check size of statistical counters. > +/* TODO: check size of statistical counters. */ > s->stats_size = 80; > -// TODO: check extended tcb support. > +/* TODO: check extended tcb support. */ > s->has_extended_tcb_support = 1; > break; > case i82551: > pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_INTEL_82551IT); > /* Revision ID: 0x0f, 0x10. */ > PCI_CONFIG_8(PCI_REVISION_ID, 0x0f); > -// TODO: check size of statistical counters. > +/* TODO: check size of statistical counters. */ > s->stats_size = 80; > s->has_extended_tcb_support = 1; > break; > @@ -572,7 +578,7 @@ static void pci_reset(EEPRO100State * s) > PCI_CONFIG_16(PCI_STATUS, PCI_STATUS_DEVSEL_MEDIUM | >PCI_STATUS_FAST_BACK | > PCI_STATUS_CAP_LIST); > PCI_CONFIG_8(PCI_REVISION_ID, 0x08); > -// TODO: Windows wants revision id 0x0c. > +/* TODO: Windows wants revision id 0x0c. */ > PCI_CONFIG_8(PCI_REVISION_ID, 0x0c); > #if EEPROM_SIZE > 0 > PCI_CONFIG_16(PCI_SUBSYSTEM_VENDOR_ID, 0x8086); > @@ -590,7 +596,7 @@ static void pci_reset(EEPRO100State * s) > s->has_extended_tcb_suppo
[Qemu-devel] Re: [PATCHv3 17/20] eepro100: New function for reading command block
On Tue, Mar 02, 2010 at 10:37:57PM +0100, Stefan Weil wrote: > Move code which reads the command block to the > new function read_cb. The patch also fixes some > endianess issues related to the command block > and moves declarations of local variables to > the beginning of the block. > > Signed-off-by: Stefan Weil > --- > hw/eepro100.c | 42 -- > 1 files changed, 28 insertions(+), 14 deletions(-) > > diff --git a/hw/eepro100.c b/hw/eepro100.c > index f5aa306..e10ce62 100644 > --- a/hw/eepro100.c > +++ b/hw/eepro100.c > @@ -798,6 +798,16 @@ static void dump_statistics(EEPRO100State * s) > //~ missing("CU dump statistical counters"); > } > > +static void read_cb(EEPRO100State *s) > +{ > +cpu_physical_memory_read(s->cb_address, (uint8_t *) &s->tx, > sizeof(s->tx)); > +s->tx.status = le16_to_cpu(s->tx.status); > +s->tx.command = le16_to_cpu(s->tx.command); > +s->tx.link = le32_to_cpu(s->tx.link); > +s->tx.tbd_array_addr = le32_to_cpu(s->tx.tbd_array_addr); > +s->tx.tcb_bytes = le16_to_cpu(s->tx.tcb_bytes); > +} > + > static void tx_command(EEPRO100State *s) > { > uint32_t tbd_array = le32_to_cpu(s->tx.tbd_array_addr); > @@ -901,21 +911,25 @@ static void set_multicast_list(EEPRO100State *s) > static void action_command(EEPRO100State *s) > { > for (;;) { > -s->cb_address = s->cu_base + s->cu_offset; > -cpu_physical_memory_read(s->cb_address, (uint8_t *)&s->tx, > sizeof(s->tx)); > -uint16_t command = le16_to_cpu(s->tx.command); > -s->tx.status = le16_to_cpu(s->tx.status); > -logout("val=(cu start), status=0x%04x, command=0x%04x, > link=0x%08x\n", > - s->tx.status, command, s->tx.link); > -bool bit_el = ((command & COMMAND_EL) != 0); > -bool bit_s = ((command & COMMAND_S) != 0); > -bool bit_i = ((command & COMMAND_I) != 0); > -bool bit_nc = ((command & COMMAND_NC) != 0); > +bool bit_el; > +bool bit_s; > +bool bit_i; > +bool bit_nc; > bool success = true; > -//~ bool bit_sf = ((command & COMMAND_SF) != 0); > -uint16_t cmd = command & COMMAND_CMD; > -s->cu_offset = le32_to_cpu(s->tx.link); > -switch (cmd) { > +s->cb_address = s->cu_base + s->cu_offset; > +read_cb(s); > +bit_el = ((s->tx.command & COMMAND_EL) != 0); > +bit_s = ((s->tx.command & COMMAND_S) != 0); > +bit_i = ((s->tx.command & COMMAND_I) != 0); > +bit_nc = ((s->tx.command & COMMAND_NC) != 0); () is never needed around the assigned value. this can be fixed separately though. AFAIK you also do not need != : values are coersed by assignment to bool correctly. Again, can be fixed separately. > +#if 0 > +bool bit_sf = ((s->tx.command & COMMAND_SF) != 0); > +#endif Is the above likely to be useful? > +s->cu_offset = s->tx.link; > +TRACE(OTHER, > + logout("val=(cu start), status=0x%04x, command=0x%04x, > link=0x%08x\n", > + s->tx.status, s->tx.command, s->tx.link)); > +switch (s->tx.command & COMMAND_CMD) { > case CmdNOp: > /* Do nothing. */ > break; > -- > 1.7.0
Re: [Qemu-devel] [PATCH] qemu-img: Fix error "Unknown option 'size'" when using host_device format
Am 02.03.2010 19:52, schrieb Bitti: > qemu-kvm-0.11.0 > > When I run: > sudo qemu-img convert ubuntu-kvm/disk0.qcow2 -O host_device /dev/vg01/vm_test > > I get this: > Unknown option 'size' > > Ubuntu Bug: > https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/460542 > > Signed-off-by: Bitti This has been fixed in master by commit 0b4ce02e which is contained in 0.12, too. Kevin
[Qemu-devel] Re: [PATCH] qemu-img rebase: Add -f option
Kevin Wolf wrote: > Allow the user to specify the format of the image to rebase. > > Signed-off-by: Kevin Wolf Acked-by: Juan Quintela This is needed to use rebase with qcow2 images on block devices.