Re: [Qemu-devel] [PATCH] possible mmap regression
Hi Edgar, On Feb 16, 2008 11:07 PM, Edgar E. Iglesias <[EMAIL PROTECTED]> wrote: > > On Tue, Feb 12, 2008 at 09:42:15PM +0200, Felipe Contreras wrote: > > Hi, > > > > I don't know what I'm doing but this seems to fix the weird issue I was > > having. > > http://article.gmane.org/gmane.comp.emulators.qemu/23314 > > > > I've found out that this happens on linux 2.6.23, but not 2.6.24. > > > > Cheers. > > > > -- > > Felipe Contreras > > diff --git a/linux-user/mmap.c b/linux-user/mmap.c > > index 6292826..3050ad9 100644 > > --- a/linux-user/mmap.c > > +++ b/linux-user/mmap.c > > @@ -251,7 +251,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, > > int prot, > > especially important if qemu_host_page_size > > > qemu_real_host_page_size */ > > p = mmap(g2h(mmap_start), > > - host_len, prot, flags | MAP_FIXED, fd, host_offset); > > + host_len, prot, flags, fd, host_offset); > > if (p == MAP_FAILED) > > return -1; > > /* update start so that it points to the file position at 'offset' > > */ > > Hello, > > Sorry, but I beleive your patch will break simulations where the targets > pagesize is larger than the hosts. > > Would you mind trying the attach patched and let us know if it helps for you? > If not, it would be great if you could provide a small test case that trigs > the bug you are seeing so we can debug the problem. > > Best regards > -- > Edgar E. Iglesias > Axis Communications AB > > diff --git a/linux-user/mmap.c b/linux-user/mmap.c > index 6292826..78a8162 100644 > --- a/linux-user/mmap.c > +++ b/linux-user/mmap.c > @@ -385,6 +385,9 @@ int target_munmap(abi_ulong start, abi_ulong len) > real_end -= qemu_host_page_size; > } > > +if (start < mmap_next_start) > + mmap_next_start = start; > + > /* unmap what we can */ > if (real_start < real_end) { > ret = munmap(g2h(real_start), real_end - real_start); > I tried your patch and it still crashes. I sent the details before: http://article.gmane.org/gmane.comp.emulators.qemu/23314 http://article.gmane.org/gmane.comp.emulators.qemu/23328 Basically it was triggered by this change: http://repo.or.cz/w/qemu.git?a=commitdiff;h=edbcc0b2eb1d4caee5f293e5c79f81023f3394e2 And it happens with some recursive Makefiles stuff. Best regards. -- Felipe Contreras
Re: [Qemu-devel] [PATCH] possible mmap regression
On Wed, Feb 20, 2008 at 03:03:39PM +0200, Felipe Contreras wrote: > Hi Edgar, > > On Feb 16, 2008 11:07 PM, Edgar E. Iglesias <[EMAIL PROTECTED]> wrote: > > > > On Tue, Feb 12, 2008 at 09:42:15PM +0200, Felipe Contreras wrote: > > > Hi, > > > > > > I don't know what I'm doing but this seems to fix the weird issue I was > > > having. > > > http://article.gmane.org/gmane.comp.emulators.qemu/23314 > > > > > > I've found out that this happens on linux 2.6.23, but not 2.6.24. > > > > > > Cheers. > > > > > > -- > > > Felipe Contreras > > > diff --git a/linux-user/mmap.c b/linux-user/mmap.c > > > index 6292826..3050ad9 100644 > > > --- a/linux-user/mmap.c > > > +++ b/linux-user/mmap.c > > > @@ -251,7 +251,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, > > > int prot, > > > especially important if qemu_host_page_size > > > > qemu_real_host_page_size */ > > > p = mmap(g2h(mmap_start), > > > - host_len, prot, flags | MAP_FIXED, fd, host_offset); > > > + host_len, prot, flags, fd, host_offset); > > > if (p == MAP_FAILED) > > > return -1; > > > /* update start so that it points to the file position at > > > 'offset' */ > > > > Hello, > > > > Sorry, but I beleive your patch will break simulations where the targets > > pagesize is larger than the hosts. > > > > Would you mind trying the attach patched and let us know if it helps for > > you? > > If not, it would be great if you could provide a small test case that trigs > > the bug you are seeing so we can debug the problem. > > > > Best regards > > -- > > Edgar E. Iglesias > > Axis Communications AB > > > > diff --git a/linux-user/mmap.c b/linux-user/mmap.c > > index 6292826..78a8162 100644 > > --- a/linux-user/mmap.c > > +++ b/linux-user/mmap.c > > @@ -385,6 +385,9 @@ int target_munmap(abi_ulong start, abi_ulong len) > > real_end -= qemu_host_page_size; > > } > > > > +if (start < mmap_next_start) > > + mmap_next_start = start; > > + > > /* unmap what we can */ > > if (real_start < real_end) { > > ret = munmap(g2h(real_start), real_end - real_start); > > > > I tried your patch and it still crashes. > > I sent the details before: > http://article.gmane.org/gmane.comp.emulators.qemu/23314 > http://article.gmane.org/gmane.comp.emulators.qemu/23328 > > Basically it was triggered by this change: > http://repo.or.cz/w/qemu.git?a=commitdiff;h=edbcc0b2eb1d4caee5f293e5c79f81023f3394e2 > > And it happens with some recursive Makefiles stuff. Thanks Felipe, I was also seeing errors with that commit. Later that same evening I found a few more errors with the mmap code which tried to fix. Would you mind trying that patch too? You can find it here: http://lists.gnu.org/archive/html/qemu-devel/2008-02/msg00331.html Best regards - Edgar E. Iglesias Axis Communications AB
Re: [Qemu-devel] [PATCH] possible mmap regression
On Feb 20, 2008 3:13 PM, Edgar E. Iglesias <[EMAIL PROTECTED]> wrote: > > On Wed, Feb 20, 2008 at 03:03:39PM +0200, Felipe Contreras wrote: > > Hi Edgar, > > > > On Feb 16, 2008 11:07 PM, Edgar E. Iglesias <[EMAIL PROTECTED]> wrote: > > > > > > On Tue, Feb 12, 2008 at 09:42:15PM +0200, Felipe Contreras wrote: > > > > Hi, > > > > > > > > I don't know what I'm doing but this seems to fix the weird issue I was > > > > having. > > > > http://article.gmane.org/gmane.comp.emulators.qemu/23314 > > > > > > > > I've found out that this happens on linux 2.6.23, but not 2.6.24. > > > > > > > > Cheers. > > > > > > > > -- > > > > Felipe Contreras > > > > diff --git a/linux-user/mmap.c b/linux-user/mmap.c > > > > index 6292826..3050ad9 100644 > > > > --- a/linux-user/mmap.c > > > > +++ b/linux-user/mmap.c > > > > @@ -251,7 +251,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong > > > > len, int prot, > > > > especially important if qemu_host_page_size > > > > > qemu_real_host_page_size */ > > > > p = mmap(g2h(mmap_start), > > > > - host_len, prot, flags | MAP_FIXED, fd, host_offset); > > > > + host_len, prot, flags, fd, host_offset); > > > > if (p == MAP_FAILED) > > > > return -1; > > > > /* update start so that it points to the file position at > > > > 'offset' */ > > > > > > Hello, > > > > > > Sorry, but I beleive your patch will break simulations where the targets > > > pagesize is larger than the hosts. > > > > > > Would you mind trying the attach patched and let us know if it helps for > > > you? > > > If not, it would be great if you could provide a small test case that > > > trigs the bug you are seeing so we can debug the problem. > > > > > > Best regards > > > -- > > > Edgar E. Iglesias > > > Axis Communications AB > > > > > > diff --git a/linux-user/mmap.c b/linux-user/mmap.c > > > index 6292826..78a8162 100644 > > > --- a/linux-user/mmap.c > > > +++ b/linux-user/mmap.c > > > @@ -385,6 +385,9 @@ int target_munmap(abi_ulong start, abi_ulong len) > > > real_end -= qemu_host_page_size; > > > } > > > > > > +if (start < mmap_next_start) > > > + mmap_next_start = start; > > > + > > > /* unmap what we can */ > > > if (real_start < real_end) { > > > ret = munmap(g2h(real_start), real_end - real_start); > > > > > > > I tried your patch and it still crashes. > > > > I sent the details before: > > http://article.gmane.org/gmane.comp.emulators.qemu/23314 > > http://article.gmane.org/gmane.comp.emulators.qemu/23328 > > > > Basically it was triggered by this change: > > http://repo.or.cz/w/qemu.git?a=commitdiff;h=edbcc0b2eb1d4caee5f293e5c79f81023f3394e2 > > > > And it happens with some recursive Makefiles stuff. > > Thanks Felipe, > > I was also seeing errors with that commit. Later that same evening I found a > few more errors with the mmap code which tried to fix. Would you mind trying > that patch too? > > You can find it here: > http://lists.gnu.org/archive/html/qemu-devel/2008-02/msg00331.html Good to know I'm not the only one :) I tried your patch, I still get the crash. Best regards. -- Felipe Contreras
[Qemu-devel] file system can be writed ?
Hello, I want to produce a file system that could be written. The environment is arm (qemu-system-arm) with linux. The file system type I prefer is ext2. But I have some questions . Is QEMU could produce a file system image that could be written? If yes, what should I do ??? Thanks a lot -- Open WebMail Project (http://openwebmail.org)
[Qemu-devel] [PATCH] ide.c error checking
The attached patch makes the ide emulation actually take notice of error returns from bdrv_write and bdrv_aio_{read,write}. Ian. diff --git a/block.c b/block.c diff --git a/hw/ide.c b/hw/ide.c index 25f5b9f..370a412 100644 --- a/hw/ide.c +++ b/hw/ide.c @@ -792,6 +792,11 @@ static void ide_set_sector(IDEState *s, int64_t sector_num) } } +static void ide_rw_error(IDEState *s) { +ide_abort_command(s); +ide_set_irq(s); +} + static void ide_sector_read(IDEState *s) { int64_t sector_num; @@ -811,6 +816,10 @@ static void ide_sector_read(IDEState *s) if (n > s->req_nb_sectors) n = s->req_nb_sectors; ret = bdrv_read(s->bs, sector_num, s->io_buffer, n); +if (ret != 0) { +ide_rw_error(s); +return; +} ide_transfer_start(s, s->io_buffer, 512 * n, ide_sector_read); ide_set_irq(s); ide_set_sector(s, sector_num + n); @@ -818,6 +827,14 @@ static void ide_sector_read(IDEState *s) } } +static void ide_dma_error(IDEState *s) +{ +ide_transfer_stop(s); +s->error = ABRT_ERR; +s->status = READY_STAT | ERR_STAT; +ide_set_irq(s); +} + /* return 0 if buffer completed */ static int dma_buf_rw(BMDMAState *bm, int is_write) { @@ -866,7 +883,6 @@ static int dma_buf_rw(BMDMAState *bm, int is_write) return 1; } -/* XXX: handle errors */ static void ide_read_dma_cb(void *opaque, int ret) { BMDMAState *bm = opaque; @@ -874,6 +890,11 @@ static void ide_read_dma_cb(void *opaque, int ret) int n; int64_t sector_num; +if (ret < 0) { + ide_dma_error(s); + return; +} + n = s->io_buffer_size >> 9; sector_num = ide_get_sector(s); if (n > 0) { @@ -938,6 +959,11 @@ static void ide_sector_write(IDEState *s) if (n > s->req_nb_sectors) n = s->req_nb_sectors; ret = bdrv_write(s->bs, sector_num, s->io_buffer, n); +if (ret != 0) { + ide_rw_error(s); + return; +} + s->nsector -= n; if (s->nsector == 0) { /* no more sectors to write */ @@ -967,7 +993,6 @@ static void ide_sector_write(IDEState *s) } } -/* XXX: handle errors */ static void ide_write_dma_cb(void *opaque, int ret) { BMDMAState *bm = opaque; @@ -975,6 +1000,11 @@ static void ide_write_dma_cb(void *opaque, int ret) int n; int64_t sector_num; +if (ret < 0) { + ide_dma_error(s); + return; +} + n = s->io_buffer_size >> 9; sector_num = ide_get_sector(s); if (n > 0) {
Re: [Qemu-devel] [PATCH] bdrv_flush error handling
On Wed, Feb 20, 2008 at 03:53:46PM +, Ian Jackson wrote: Content-Description: message body text > bdrv_flush is declared to return void, but this is wrong because it > means that the implementations have nowhere to report their errors. > Indeed, the implementations generally ignore errors. > > This patch corrects this by making it return int (implicitly, either 0 > or -errno, as for other similar functions). All of the > implementations and callers are adjusted too. > > > While looking at this I was surprised to see this: > > static void scsi_write_complete(void * opaque, int ret) > ... > if (ret) { > fprintf(stderr, "scsi-disc: IO write error\n"); > exit(1); > } > > Surely that is overkill ? Yes, any i/o errors I'd expect to be propagated back up to the guest OS as the most appropriate IDE / SCSI error code. > Also, in block-raw-posix.c, raw_pwrite et al seem to return -1 on > error (the return value from write) whereas the other block read/write > methods return errno values. This is a mistake, surely ? -1 would be > -EPERM. If any of the callers did anything with these return values > you'd get incorrect error indications. > > > Finally, it would perhaps be best if the block device emulators wrote > to the qemu console to complain if they give write errors. Otherwise > the errno value and other important information will be lost, which > makes debugging hard. If by 'qemu console' you mean stderr, then fine, but please don't spew log messages to the monitor console, because that'll make it very hard to interact with reliably from management tools. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
Re: [Qemu-devel] [PATCH] bdrv_flush error handling
Daniel P. Berrange writes ("Re: [Qemu-devel] [PATCH] bdrv_flush error handling"): > On Wed, Feb 20, 2008 at 03:53:46PM +, Ian Jackson wrote: > > Finally, it would perhaps be best if the block device emulators wrote > > to the qemu console to complain if they give write errors. Otherwise > > the errno value and other important information will be lost, which > > makes debugging hard. > > If by 'qemu console' you mean stderr, then fine, but please don't > spew log messages to the monitor console, because that'll make it > very hard to interact with reliably from management tools. Is it permitted, then, to generally print to qemu's stderr from inside a device model ? This seems to be done quite a bit during initialisation, and quite a bit in various rather less common kinds of device, but not routinely in the main emulations. Perhaps it would be better to invent a `logprintf' function to make it easier to redirect these kind of messages later if that turns out to be desirable ? Ian.
Re: [Qemu-devel] [PATCH] bdrv_flush error handling
> > Finally, it would perhaps be best if the block device emulators wrote > > to the qemu console to complain if they give write errors. Otherwise > > the errno value and other important information will be lost, which > > makes debugging hard. > > If by 'qemu console' you mean stderr, then fine, but please don't > spew log messages to the monitor console, because that'll make it > very hard to interact with reliably from management tools. Actually I think a better default would be for qemu to die right there and then. If the host is getting IO errors then something is badly wrong, and you're probably screwed anyway. Paul
Re: [Qemu-devel] precompiled qemu-system-x86_64 is 32 bit instead of 64bit
- Original Message - From: "Ralf Baerwaldt" <[EMAIL PROTECTED]> To: Sent: 19.02.2008 18:09 Subject: [Qemu-devel] precompiled qemu-system-x86_64 is 32 bit instead of 64bit I downloaded http://fabrice.bellard.free.fr/qemu/qemu-0.9.1-i386.tar.gz and I'm using an AMD Opteron. Is this version the correct one for my system ? If You want qemu for your amd64 linux system, You should compile it yourself or install from Your linux distribution repository. This site (http://fabrice.bellard.free.fr/qemu/download.html) doesn't contain precompiled linux binaries except for i386 platform. However You could execute i386 binaries on amd64 linux platform. :) Sergey Bychkow ICQ: 21014758 FTN: 2:450/118.55
[Qemu-devel] [PATCH] wrap up use of dangerous ctype.h macros
The macros isupper, isspace, tolower, and so forth (from ) are defined to take an int containing the value of an unsigned char, or EOF. This means that you mustn't write: char *p; ... if (isspace(*p)) { ... If *p is a top-bit-set character, and your host architecture has signed chars by default (most do), then the result is passing a negative number to isspace, which is not allowed. glibc permits this but not all libcs do, and it is wrong according to C89 and C99. The correct use is: if (isspace((unsigned char)*p)) { ... This is rather unweildy so I in the attached patch have invented a CTYPE macro which takes some but not all of the pain out of it. I haven't checked the context of every use I changed to confirm that the char* is actually a char* rather than an unsigned char*, and I did not check whether the data is static data which is part of qemu and guaranteed to contain only 7-bit characters - so technically it is possible that some of these changes are unnecessary. However they are all correct changes and changing it everywhere sets the right example. Ian. diff --git a/audio/audio.c b/audio/audio.c index 2ccef58..653e196 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -235,7 +235,7 @@ static char *audio_alloc_prefix (const char *s) strcat (r, s); for (i = 0; i < len; ++i) { -u[i] = toupper (u[i]); +u[i] = CTYPE(toupper, u[i]); } } return r; @@ -489,7 +489,7 @@ static void audio_process_options (const char *prefix, /* copy while upper-casing, including trailing zero */ for (i = 0; i <= preflen; ++i) { -optname[i + sizeof (qemu_prefix) - 1] = toupper (prefix[i]); +optname[i + sizeof (qemu_prefix) - 1] = CTYPE(toupper, prefix[i]); } strcat (optname, "_"); strcat (optname, opt->name); diff --git a/block-vvfat.c b/block-vvfat.c index ad81425..7fe7e6a 100644 --- a/block-vvfat.c +++ b/block-vvfat.c @@ -1476,7 +1476,7 @@ static int parse_short_name(BDRVVVFATState* s, if (direntry->name[i] <= ' ' || direntry->name[i] > 0x7f) return -1; else if (s->downcase_short_names) - lfn->name[i] = tolower(direntry->name[i]); + lfn->name[i] = CTYPE(tolower,direntry->name[i]); else lfn->name[i] = direntry->name[i]; } @@ -1489,7 +1489,7 @@ static int parse_short_name(BDRVVVFATState* s, if (direntry->extension[j] <= ' ' || direntry->extension[j] > 0x7f) return -2; else if (s->downcase_short_names) - lfn->name[i + j] = tolower(direntry->extension[j]); + lfn->name[i + j] = CTYPE(tolower,direntry->extension[j]); else lfn->name[i + j] = direntry->extension[j]; } diff --git a/cutils.c b/cutils.c index 9ef2fa6..e8e022c 100644 --- a/cutils.c +++ b/cutils.c @@ -72,7 +72,7 @@ int stristart(const char *str, const char *val, const char **ptr) p = str; q = val; while (*q != '\0') { -if (toupper(*p) != toupper(*q)) +if (CTYPE(toupper,*p) != CTYPE(toupper,*q)) return 0; p++; q++; diff --git a/monitor.c b/monitor.c index 63d9fad..8e2187f 100644 --- a/monitor.c +++ b/monitor.c @@ -1779,7 +1779,7 @@ static void next(void) { if (pch != '\0') { pch++; -while (isspace(*pch)) +while (CTYPE(isspace,*pch)) pch++; } } @@ -1838,7 +1838,7 @@ static int64_t expr_unary(void) *q++ = *pch; pch++; } -while (isspace(*pch)) +while (CTYPE(isspace,*pch)) pch++; *q = 0; ret = get_monitor_def(®, buf); @@ -1863,7 +1863,7 @@ static int64_t expr_unary(void) expr_error("invalid char in expression"); } pch = p; -while (isspace(*pch)) +while (CTYPE(isspace,*pch)) pch++; break; } @@ -1957,7 +1957,7 @@ static int get_expr(int64_t *pval, const char **pp) *pp = pch; return -1; } -while (isspace(*pch)) +while (CTYPE(isspace,*pch)) pch++; *pval = expr_sum(); *pp = pch; @@ -1972,7 +1972,7 @@ static int get_str(char *buf, int buf_size, const char **pp) q = buf; p = *pp; -while (isspace(*p)) +while (CTYPE(isspace,*p)) p++; if (*p == '\0') { fail: @@ -2017,7 +2017,7 @@ static int get_str(char *buf, int buf_size, const char **pp) } p++; } else { -while (*p != '\0' && !isspace(*p)) { +while (*p != '\0' && !CTYPE(isspace,*p)) { if ((q - buf) < buf_size - 1) { *q++ = *p; } @@ -2052,12 +2052,12 @@ static void monitor_handle_command(const char *cmdline) /* extract the command name */ p = cmdline; q = cmdname; -while (isspace(*p)) +while (CTYPE(isspace,*p))
[Qemu-devel] [PATCH] bdrv_flush error handling
bdrv_flush is declared to return void, but this is wrong because it means that the implementations have nowhere to report their errors. Indeed, the implementations generally ignore errors. This patch corrects this by making it return int (implicitly, either 0 or -errno, as for other similar functions). All of the implementations and callers are adjusted too. While looking at this I was surprised to see this: static void scsi_write_complete(void * opaque, int ret) ... if (ret) { fprintf(stderr, "scsi-disc: IO write error\n"); exit(1); } Surely that is overkill ? Also, in block-raw-posix.c, raw_pwrite et al seem to return -1 on error (the return value from write) whereas the other block read/write methods return errno values. This is a mistake, surely ? -1 would be -EPERM. If any of the callers did anything with these return values you'd get incorrect error indications. Finally, it would perhaps be best if the block device emulators wrote to the qemu console to complain if they give write errors. Otherwise the errno value and other important information will be lost, which makes debugging hard. Thanks, Ian. diff --git a/block-qcow.c b/block-qcow.c index 6ec625d..d53d1d0 100644 --- a/block-qcow.c +++ b/block-qcow.c @@ -879,10 +879,10 @@ static int qcow_write_compressed(BlockDriverState *bs, int64_t sector_num, return 0; } -static void qcow_flush(BlockDriverState *bs) +static int qcow_flush(BlockDriverState *bs) { BDRVQcowState *s = bs->opaque; -bdrv_flush(s->hd); +return bdrv_flush(s->hd); } static int qcow_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) diff --git a/block-qcow2.c b/block-qcow2.c index 577210b..9bad090 100644 --- a/block-qcow2.c +++ b/block-qcow2.c @@ -1228,10 +1228,10 @@ static int qcow_write_compressed(BlockDriverState *bs, int64_t sector_num, return 0; } -static void qcow_flush(BlockDriverState *bs) +static int qcow_flush(BlockDriverState *bs) { BDRVQcowState *s = bs->opaque; -bdrv_flush(s->hd); +return bdrv_flush(s->hd); } static int qcow_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) diff --git a/block-raw-posix.c b/block-raw-posix.c index 28aaf02..45d0a93 100644 --- a/block-raw-posix.c +++ b/block-raw-posix.c @@ -539,10 +539,12 @@ static int raw_create(const char *filename, int64_t total_size, return 0; } -static void raw_flush(BlockDriverState *bs) +static int raw_flush(BlockDriverState *bs) { BDRVRawState *s = bs->opaque; -fsync(s->fd); +if (fsync(s->fd)) +return errno; +return 0; } BlockDriver bdrv_raw = { diff --git a/block-raw-win32.c b/block-raw-win32.c index 43d3f6c..b86c66e 100644 --- a/block-raw-win32.c +++ b/block-raw-win32.c @@ -269,10 +269,14 @@ static void raw_aio_cancel(BlockDriverAIOCB *blockacb) } #endif /* #if 0 */ -static void raw_flush(BlockDriverState *bs) +static int raw_flush(BlockDriverState *bs) { BDRVRawState *s = bs->opaque; -FlushFileBuffers(s->hfile); +int ret; +ret = FlushFileBuffers(s->hfile); +if (ret) + return -EIO; +return 0; } static void raw_close(BlockDriverState *bs) diff --git a/block-vmdk.c b/block-vmdk.c index b6ec89e..4cacc6a 100644 --- a/block-vmdk.c +++ b/block-vmdk.c @@ -815,10 +815,10 @@ static void vmdk_close(BlockDriverState *bs) vmdk_parent_close(s->hd); } -static void vmdk_flush(BlockDriverState *bs) +static int vmdk_flush(BlockDriverState *bs) { BDRVVmdkState *s = bs->opaque; -bdrv_flush(s->hd); +return bdrv_flush(s->hd); } BlockDriver bdrv_vmdk = { diff --git a/block.c b/block.c index 0f8ad7b..b1edd22 100644 --- a/block.c +++ b/block.c @@ -865,12 +865,14 @@ const char *bdrv_get_device_name(BlockDriverState *bs) return bs->device_name; } -void bdrv_flush(BlockDriverState *bs) +int bdrv_flush(BlockDriverState *bs) { -if (bs->drv->bdrv_flush) -bs->drv->bdrv_flush(bs); -if (bs->backing_hd) -bdrv_flush(bs->backing_hd); +int ret = 0; +if (bs->drv->bdrv_flush) +ret = bs->drv->bdrv_flush(bs); +if (!ret && bs->backing_hd) +ret = bdrv_flush(bs->backing_hd); +return ret; } #ifndef QEMU_IMG diff --git a/block.h b/block.h index b730505..861a65b 100644 --- a/block.h +++ b/block.h @@ -98,7 +98,7 @@ void qemu_aio_wait_end(void); int qemu_key_check(BlockDriverState *bs, const char *name); /* Ensure contents are flushed to disk. */ -void bdrv_flush(BlockDriverState *bs); +int bdrv_flush(BlockDriverState *bs); #define BDRV_TYPE_HD 0 #define BDRV_TYPE_CDROM 1 diff --git a/block_int.h b/block_int.h index 137000e..796ce29 100644 --- a/block_int.h +++ b/block_int.h @@ -42,7 +42,7 @@ struct BlockDriver { void (*bdrv_close)(BlockDriverState *bs); int (*bdrv_create)(const char *filename, int64_t total_sectors, const char *backing_file, int flags); -void (*bdrv_flush)(BlockDriverState *bs); +int (*bdrv_flush)
Re: [Qemu-devel] [PATCH] bdrv_flush error handling
Paul Brook wrote: Finally, it would perhaps be best if the block device emulators wrote to the qemu console to complain if they give write errors. Otherwise the errno value and other important information will be lost, which makes debugging hard. If by 'qemu console' you mean stderr, then fine, but please don't spew log messages to the monitor console, because that'll make it very hard to interact with reliably from management tools. Actually I think a better default would be for qemu to die right there and then. If the host is getting IO errors then something is badly wrong, and you're probably screwed anyway. If you're passing through a real disk or volume, this denies the guest the possibility of recover y (for example, if it is running a RAID setup over multiple volumes, or if you are testing the guest's ability to recover from errors). For non-raw formats, you can pass through errors on data, but it is impossible to recover on metadata errors, so dying on I/O error should be fine. -- Any sufficiently difficult bug is indistinguishable from a feature.
Re: [Qemu-devel] [PATCH] bdrv_flush error handling
Avi Kivity writes ("Re: [Qemu-devel] [PATCH] bdrv_flush error handling"): > For non-raw formats, you can pass through errors on data, but it is > impossible to recover on metadata errors, so dying on I/O error should > be fine. You mean metadata write errors I assume. I don't see why a metadata read error is any worse than any other read error. The guest will often prefer to be told that the volume was broken and then to be denied the ability to continue accessing it. Who knows what the guest thinks of the device ? Perhaps it's an unimportant peripheral, or simulated network block device, used only for data interchange. Write errors for non-raw formats can easily be caused by a disk full situation on the host. Killing the guest hard is unfriendly for that situation. Ian.
Re: [Qemu-devel] [PATCH] bdrv_flush error handling
Paul Brook writes ("Re: [Qemu-devel] [PATCH] bdrv_flush error handling"): > Disk full is a fundamentally unfriendly situation to be in. There is no good > answer. Reporting errors back to the host has its own set of problems. Many > guest OS effectively just lock up when this occurs. I think that's fine, surely ? A locked up guest isn't very nice but it's better than a guest shot dead. Ian.
Re: [Qemu-devel] [PATCH] bdrv_flush error handling
> Write errors for non-raw formats can easily be caused by a disk full > situation on the host. Killing the guest hard is unfriendly for that > situation. Disk full is a fundamentally unfriendly situation to be in. There is no good answer. Reporting errors back to the host has its own set of problems. Many guest OS effectively just lock up when this occurs. Paul
Re: [Qemu-devel] [PATCH] APIC: add NMI and SMI IPI support
Robi Yagel wrote: > Hello, > Thanks for the patch, Nice to hear that there is interest in this. :) > if you can, please advice on the proper place to add > periodic generation of SMI/NMIs in order to simulate, e.g., a watchdog (and > the > needed parameters - except for CPU_INTERRUPT_NMI...) Yeah, I also thought about NMI watchdog emulation while adding CPU_INTERRUPT_NMI, but it took a bit more effort to understand what pieces are missing. It should be a fairly useful feature for testing NMI interaction with new kernel code and maybe even catching nasty races that way (whenever you happen to work on such things, like I do ;) ). Also, we need NMI delivery for custom hardware emulation here. So there might be broader use for CPU_INTERRUPT_NMI in the future. However, find some experimental watchdog-enabler patch below. It allows to use nmi_watchdog=1 (i.e. the IO-APIC variant) with Linux, without any visible regression of normal IRQ delivery (but I only lightly tested it on top of the kvm-userspace qemu, that's why "experimental"). A performance counter based NMI watchdog is surely doable as well, but it takes a bit more effort. You would have to decide first what perf-counter features should be emulated and how cleanly, even if you only want to (mis-)use it for watchdog services. I'm lacking time for this right now. Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux --- hw/apic.c | 38 +- hw/pc.c |2 +- hw/pc.h |3 +++ 3 files changed, 37 insertions(+), 6 deletions(-) Index: b/hw/apic.c === --- a/hw/apic.c +++ b/hw/apic.c @@ -166,6 +166,37 @@ static inline void reset_bit(uint32_t *t tab[i] &= ~mask; } +void apic_local_deliver(CPUState *env, int vector) +{ +APICState *s = env->apic_state; +uint32_t lvt = s->lvt[vector]; +int trigger_mode; + +if (lvt & APIC_LVT_MASKED) +return; + +switch ((lvt >> 8) & 7) { +case APIC_DM_SMI: +cpu_interrupt(env, CPU_INTERRUPT_SMI); +break; + +case APIC_DM_NMI: +cpu_interrupt(env, CPU_INTERRUPT_NMI); +break; + +case APIC_DM_EXTINT: +cpu_interrupt(env, CPU_INTERRUPT_HARD); +break; + +case APIC_DM_FIXED: +trigger_mode = APIC_TRIGGER_EDGE; +if ((vector == APIC_LVT_LINT0 || vector == APIC_LVT_LINT1) && +(lvt & APIC_LVT_LEVEL_TRIGGER)) +trigger_mode = APIC_TRIGGER_LEVEL; +apic_set_irq(s, lvt & 0xff, trigger_mode); +} +} + #define foreach_apic(apic, deliver_bitmask, code) \ {\ int __i, __j, __mask;\ @@ -504,8 +535,7 @@ int apic_accept_pic_intr(CPUState *env) if (s->id == 0 && ((s->apicbase & MSR_IA32_APICBASE_ENABLE) == 0 || - ((lvt0 & APIC_LVT_MASKED) == 0 && - ((lvt0 >> 8) & 0x7) == APIC_DM_EXTINT))) + (lvt0 & APIC_LVT_MASKED) == 0)) return 1; return 0; @@ -556,9 +586,7 @@ static void apic_timer(void *opaque) { APICState *s = opaque; -if (!(s->lvt[APIC_LVT_TIMER] & APIC_LVT_MASKED)) { -apic_set_irq(s, s->lvt[APIC_LVT_TIMER] & 0xff, APIC_TRIGGER_EDGE); -} +apic_local_deliver(s->cpu_env, APIC_LVT_TIMER); apic_timer_update(s, s->next_time); } Index: b/hw/pc.c === --- a/hw/pc.c +++ b/hw/pc.c @@ -115,7 +115,7 @@ static void pic_irq_request(void *opaque { CPUState *env = opaque; if (level && apic_accept_pic_intr(env)) -cpu_interrupt(env, CPU_INTERRUPT_HARD); +apic_local_deliver(env, APIC_LINT0); } /* PC cmos mappings */ Index: b/hw/pc.h === --- a/hw/pc.h +++ b/hw/pc.h @@ -39,8 +39,11 @@ void irq_info(void); /* APIC */ typedef struct IOAPICState IOAPICState; +#define APIC_LINT0 3 + int apic_init(CPUState *env); int apic_accept_pic_intr(CPUState *env); +void apic_local_deliver(CPUState *env, int vector); int apic_get_interrupt(CPUState *env); IOAPICState *ioapic_init(void); void ioapic_set_irq(void *opaque, int vector, int level);
[Qemu-devel] qemu/tcg tcg-op.h
CVSROOT:/cvsroot/qemu Module name:qemu Changes by: Blue Swirl 08/02/20 18:01:24 Modified files: tcg: tcg-op.h Log message: Fix andi, optimize addi and subi zero cases CVSWeb URLs: http://cvs.savannah.gnu.org/viewcvs/qemu/tcg/tcg-op.h?cvsroot=qemu&r1=1.3&r2=1.4
Re: [Qemu-devel] [PATCH] bdrv_flush error handling
> Is savevm-upon-disk-full a realistic prospect? Not particularly useful. If you're run out of disk space, chances are that savevm will also fail. Paul
Re: [Qemu-devel] [PATCH] bdrv_flush error handling
Ian Jackson wrote: > Paul Brook writes ("Re: [Qemu-devel] [PATCH] bdrv_flush error handling"): > > Disk full is a fundamentally unfriendly situation to be in. There is no > > good > > answer. Reporting errors back to the host has its own set of problems. Many > > guest OS effectively just lock up when this occurs. > > I think that's fine, surely ? A locked up guest isn't very nice but > it's better than a guest shot dead. Well, a guest which receives an IDE write error might do things like mark parts of the device bad, to avoiding writing to those parts. If the guest is running software RAID, for example, it will radically change its behaviour in response to those errors. Sometimes that's what you want, but sometimes it is really unwanted. If the host runs out of disk space, ideally you might want to suspend the guest until you can free up host disk space (or move to another host), then resume the guest, perhaps manually. Is savevm-upon-disk-full a realistic prospect? -- Jamie
Re: [Qemu-devel] [PATCH] bdrv_flush error handling
Paul Brook wrote: > > Is savevm-upon-disk-full a realistic prospect? > > Not particularly useful. If you're run out of disk space, chances are that > savevm will also fail. I'm imagining (a) that the savevm space is preallocated, or (b) is on a different disk. -- Jamie
Re: [Qemu-devel] [PATCH] bdrv_flush error handling
On Wed, Feb 20, 2008 at 04:31:56PM +, Jamie Lokier wrote: > Ian Jackson wrote: > > Paul Brook writes ("Re: [Qemu-devel] [PATCH] bdrv_flush error handling"): > > > Disk full is a fundamentally unfriendly situation to be in. There is no > > > good > > > answer. Reporting errors back to the host has its own set of problems. > > > Many > > > guest OS effectively just lock up when this occurs. > > > > I think that's fine, surely ? A locked up guest isn't very nice but > > it's better than a guest shot dead. > > Well, a guest which receives an IDE write error might do things like > mark parts of the device bad, to avoiding writing to those parts. If > the guest is running software RAID, for example, it will radically > change its behaviour in response to those errors. > > Sometimes that's what you want, but sometimes it is really unwanted. > If the host runs out of disk space, ideally you might want to suspend > the guest until you can free up host disk space (or move to another > host), then resume the guest, perhaps manually. > > Is savevm-upon-disk-full a realistic prospect? In the 'out of disk space' scenario you wouldn't need to save the guest - merely stop its CPU. This gives the host admin the opportunity to hot-add storage to the host & then resume execution, or to kill the VM, or to free enough space to save the VM to disk / live migrate it to another host. Shooting it dead on any I/O error doesn't give the host admin any choices at all Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
Re: [Qemu-devel] [PATCH] bdrv_flush error handling
Daniel P. Berrange wrote: On Wed, Feb 20, 2008 at 04:31:56PM +, Jamie Lokier wrote: Ian Jackson wrote: Paul Brook writes ("Re: [Qemu-devel] [PATCH] bdrv_flush error handling"): Disk full is a fundamentally unfriendly situation to be in. There is no good answer. Reporting errors back to the host has its own set of problems. Many guest OS effectively just lock up when this occurs. I think that's fine, surely ? A locked up guest isn't very nice but it's better than a guest shot dead. Well, a guest which receives an IDE write error might do things like mark parts of the device bad, to avoiding writing to those parts. If the guest is running software RAID, for example, it will radically change its behaviour in response to those errors. Sometimes that's what you want, but sometimes it is really unwanted. If the host runs out of disk space, ideally you might want to suspend the guest until you can free up host disk space (or move to another host), then resume the guest, perhaps manually. Is savevm-upon-disk-full a realistic prospect? In the 'out of disk space' scenario you wouldn't need to save the guest - merely stop its CPU. This gives the host admin the opportunity to hot-add storage to the host & then resume execution, or to kill the VM, or to free enough space to save the VM to disk / live migrate it to another host. I agree. Stopping the CPUs and spitting out a big fat warning message would be the best thing to do. For the average user, this would give the opportunity to free up some space if possible. Regards, Anthony Liguori Shooting it dead on any I/O error doesn't give the host admin any choices at all Dan.
Re: [Qemu-devel] [PATCH] bdrv_flush error handling
On Wednesday 20 February 2008 01:01:33 pm Anthony Liguori wrote: > Daniel P. Berrange wrote: > > On Wed, Feb 20, 2008 at 04:31:56PM +, Jamie Lokier wrote: > >> Ian Jackson wrote: > >>> Paul Brook writes ("Re: [Qemu-devel] [PATCH] bdrv_flush error handling"): > Disk full is a fundamentally unfriendly situation to be in. There is > no good answer. Reporting errors back to the host has its own set of > problems. Many guest OS effectively just lock up when this occurs. > >>> > >>> I think that's fine, surely ? A locked up guest isn't very nice but > >>> it's better than a guest shot dead. > >> > >> Well, a guest which receives an IDE write error might do things like > >> mark parts of the device bad, to avoiding writing to those parts. If > >> the guest is running software RAID, for example, it will radically > >> change its behaviour in response to those errors. > >> > >> Sometimes that's what you want, but sometimes it is really unwanted. > >> If the host runs out of disk space, ideally you might want to suspend > >> the guest until you can free up host disk space (or move to another > >> host), then resume the guest, perhaps manually. > >> > >> Is savevm-upon-disk-full a realistic prospect? > > > > In the 'out of disk space' scenario you wouldn't need to save the guest - > > merely stop its CPU. This gives the host admin the opportunity to hot-add > > storage to the host & then resume execution, or to kill the VM, or to > > free enough space to save the VM to disk / live migrate it to another > > host. > > I agree. Stopping the CPUs and spitting out a big fat warning message > would be the best thing to do. For the average user, this would give > the opportunity to free up some space if possible. agreed. > > Regards, > > Anthony Liguori > > > Shooting it dead on any I/O error doesn't give the host admin any choices > > at all > > > > Dan.
[Qemu-devel] [PATCH] Rework local IRQ delivery for APICs (was:[PATCH] APIC: add NMI and SMI IPI support)
Jan Kiszka wrote: > However, find some experimental watchdog-enabler patch below. It allows > to use nmi_watchdog=1 (i.e. the IO-APIC variant) with Linux, without any > visible regression of normal IRQ delivery (but I only lightly tested it > on top of the kvm-userspace qemu, that's why "experimental"). As I was lacking NMIs on CPUs > 0, I thought about IRQ delivery again and came to the conclusion, that we may better broadcast the PIC IRQ to all APICs. After fixing the current APIC initialization, this actually works as desired. Updated patch below. Jan --- hw/apic.c | 63 ++ hw/pc.c | 15 ++ hw/pc.h |3 ++ 3 files changed, 57 insertions(+), 24 deletions(-) Index: b/hw/apic.c === --- a/hw/apic.c +++ b/hw/apic.c @@ -166,6 +166,37 @@ static inline void reset_bit(uint32_t *t tab[i] &= ~mask; } +void apic_local_deliver(CPUState *env, int vector) +{ +APICState *s = env->apic_state; +uint32_t lvt = s->lvt[vector]; +int trigger_mode; + +if (lvt & APIC_LVT_MASKED) +return; + +switch ((lvt >> 8) & 7) { +case APIC_DM_SMI: +cpu_interrupt(env, CPU_INTERRUPT_SMI); +break; + +case APIC_DM_NMI: +cpu_interrupt(env, CPU_INTERRUPT_NMI); +break; + +case APIC_DM_EXTINT: +cpu_interrupt(env, CPU_INTERRUPT_HARD); +break; + +case APIC_DM_FIXED: +trigger_mode = APIC_TRIGGER_EDGE; +if ((vector == APIC_LVT_LINT0 || vector == APIC_LVT_LINT1) && +(lvt & APIC_LVT_LEVEL_TRIGGER)) +trigger_mode = APIC_TRIGGER_LEVEL; +apic_set_irq(s, lvt & 0xff, trigger_mode); +} +} + #define foreach_apic(apic, deliver_bitmask, code) \ {\ int __i, __j, __mask;\ @@ -502,10 +533,8 @@ int apic_accept_pic_intr(CPUState *env) lvt0 = s->lvt[APIC_LVT_LINT0]; -if (s->id == 0 && -((s->apicbase & MSR_IA32_APICBASE_ENABLE) == 0 || - ((lvt0 & APIC_LVT_MASKED) == 0 && - ((lvt0 >> 8) & 0x7) == APIC_DM_EXTINT))) +if ((s->apicbase & MSR_IA32_APICBASE_ENABLE) == 0 || +(lvt0 & APIC_LVT_MASKED) == 0) return 1; return 0; @@ -556,9 +585,7 @@ static void apic_timer(void *opaque) { APICState *s = opaque; -if (!(s->lvt[APIC_LVT_TIMER] & APIC_LVT_MASKED)) { -apic_set_irq(s, s->lvt[APIC_LVT_TIMER] & 0xff, APIC_TRIGGER_EDGE); -} +apic_local_deliver(s->cpu_env, APIC_LVT_TIMER); apic_timer_update(s, s->next_time); } @@ -818,12 +845,14 @@ static void apic_reset(void *opaque) APICState *s = opaque; apic_init_ipi(s); -/* - * LINT0 delivery mode is set to ExtInt at initialization time - * typically by BIOS, so PIC interrupt can be delivered to the - * processor when local APIC is enabled. - */ -s->lvt[APIC_LVT_LINT0] = 0x700; +if (s->id == 0) { +/* + * LINT0 delivery mode on CPU #0 is set to ExtInt at initialization + * time typically by BIOS, so PIC interrupt can be delivered to the + * processor when local APIC is enabled. + */ +s->lvt[APIC_LVT_LINT0] = 0x700; +} } static CPUReadMemoryFunc *apic_mem_read[3] = { @@ -848,19 +877,13 @@ int apic_init(CPUState *env) if (!s) return -1; env->apic_state = s; -apic_init_ipi(s); s->id = last_apic_id++; env->cpuid_apic_id = s->id; s->cpu_env = env; s->apicbase = 0xfee0 | (s->id ? 0 : MSR_IA32_APICBASE_BSP) | MSR_IA32_APICBASE_ENABLE; -/* - * LINT0 delivery mode is set to ExtInt at initialization time - * typically by BIOS, so PIC interrupt can be delivered to the - * processor when local APIC is enabled. - */ -s->lvt[APIC_LVT_LINT0] = 0x700; +apic_reset(s); /* XXX: mapping more APICs at the same memory location */ if (apic_io_memory == 0) { Index: b/hw/pc.c === --- a/hw/pc.c +++ b/hw/pc.c @@ -113,9 +113,16 @@ int cpu_get_pic_interrupt(CPUState *env) static void pic_irq_request(void *opaque, int irq, int level) { -CPUState *env = opaque; -if (level && apic_accept_pic_intr(env)) -cpu_interrupt(env, CPU_INTERRUPT_HARD); +CPUState *env = first_cpu; + +if (!level) +return; + +while (env) { +if (apic_accept_pic_intr(env)) +apic_local_deliver(env, APIC_LINT0); +env = env->next_cpu; +} } /* PC cmos mappings */ @@ -841,7 +848,7 @@ static void pc_init1(int ram_size, int v if (linux_boot) load_linux(kernel_filename, initrd_filename, kernel_cmdline); -cpu_irq = qemu_allocate_irqs(pic_irq_request, first_cpu, 1); +cpu_irq = qemu_allocate_irqs(pic_irq_request, NULL, 1); i8259 = i8259_init(cpu_irq[0]); ferr_irq = i8259[13]; Index: b/hw/pc.h =
[Qemu-devel] FC7 and qemu + Windows server 2003
Hi All, I am having my host OS as Fedora 7 and my guest OS is Windows Server 2003. I want to setup Network for Windows Server. I tried creating Network bridge and tap0 device. Also i tried with VDE method for network setup. Can someone please let me know what steps should i follow? Also Is it specific to Network chip. I am having RTL8201. This model is not supported with qemu. How should i go with this ? Thanks in Anticipation Thanks and Regards, Kaushik Chat on a cool, new interface. No download required. Go to http://in.messenger.yahoo.com/webmessengerpromo.php
Re: [Qemu-devel] file system can be writed ?
Hi... On Wed, Feb 20, 2008 at 9:13 PM, b93049 <[EMAIL PROTECTED]> wrote: > Is QEMU could produce a file system image that could be written? > If yes, what should I do ??? Qemu doesn't deal directly with filesystem. Via qemu-img, the only thing you can do is create empty disk image (be it raw type, cow, qcow and so on). To format it, loopback mount the disk image and use mkfs to format. regards, Mulyadi.
Re: [Qemu-devel] [PATCH] bdrv_flush error handling
Sometimes the guest intentionally seeks the error. Example? TrueCrypt 5.0 supports encryption of the full system disk. To get the real size of the disk, the truecrypt driver queries the number of blocks of the disk, but then tries to read after the last block, until it gets an error. Qemu hangs in this operation. So, for me, the blockdriver has to give the errors back to the guest (in this case, reading behind eof). In no cases, qemu should die because of that. Tom PS: I had to patch truecrypt because of this qemu feature.