Re: [Qemu-devel] "Hot" resizing QCow2 image using a simple python script
Le 29/12/2009 21:49, Kevin Wolf a écrit : > You need to really resize the L1 table and not just extend the l1_size in the > qcow2 header. And don't use the script with snapshots. VM state is saved > after the end of the virtual disk, so if you extend the disk it might overlap > the VM state (you would need to move the VM state to make it safe with > snapshots). Ok. > The right way would be a patch to qemu rather than an external python script > anyway. Doing it correctly would probably even be easier this way because > things like a function for growing the L1 table already exist. > Yeah, I was just experimenting with this script. The second version I tried of the script I wrote was copying old L1 then padding to l1_table_size, and shifting all others offsets. Are there other things to care about, like L2 table, and refcount table ? Thanks, -- Laurent Coustet
Re: [Qemu-devel] "Hot" resizing QCow2 image using a simple python script
Le 29/12/2009 21:49, Kevin Wolf a écrit : The right way would be a patch to qemu rather than an external python script anyway. I've seen the code on git a little bit, seems rather simple to me, but what do you think is the better way to integrate such a fonctionality ? New command ? The best approch for me I think is to extend "convert". Example: qemu-img -f qcow2 -O qcow2 original5G.qcow2 output10G.qcow2 +5G Thanks, -- Laurent coustet
Re: [Qemu-devel] [PATCH] add "info ioapic" monitor command
On Tue, Dec 29, 2009 at 08:00:09PM -0600, Anthony Liguori wrote: > On 12/24/2009 09:02 AM, Gleb Natapov wrote: > >Knowing ioapic configuration is very useful for the poor soles > >how need to debug guest occasionally. > > Can this be implemented in terms of VMState? > What do you mean and why it whould be any better? -- Gleb.
Re: [Qemu-devel] Re: Help on "Could not initialize SDL - exiting"
On Sun, Dec 27, 2009 at 05:35:56AM -0300, Dejun.Liu wrote:
> Hi,
>
> below is my DISPLAY info:
> echo ${DISPLAY}
> :0.0
>
> and i have already added localhost to xhost with the command:
> xhost localhost
>
> but the error continue the same.
>
> r...@dejunliu-desktop:~# echo ${DISPLAY}
> :0.0
> r...@dejunliu-desktop:~# xhost localhost
> localhost being added to access control list
> r...@dejunliu-desktop:~# qemu-system-arm -M versatilepb -usb -usbdevice
> wacom-tablet -show-cursor -m 64
> -kernel
> /home/protocol/study/OE/angstrom-dev/deploy/glibc/images/qemuarm/zImage-2.6.25+2.6.26-rc4-r6-qemuarm.bin
> -drive
> file=/home/protocol/study/OE/angstrom-dev/deploy/glibc/images/qemuarm/Angstrom-x11-image-glibc-ipk-2009.X-stable-qemuarm.rootfs.ext3
> -append "root=/dev/sda"
> Could not initialize SDL - exiting
>
> so ,is my configure right?
>
> cheers
>
> Steven Liu
>
>
> On Tue, 2009-12-29 at 12:18 +0200, Michael S. Tsirkin wrote:
> > No, I mean DISPLAY.
> >
> > On Sat, Dec 26, 2009 at 02:44:05PM -0300, Dejun.Liu wrote:
> > > Hi,
> > >
> > > do u mean:
> > > export SDL_VIDEODRIVER=x11
> > > ?
> > > i hava set sdl video driver to x11.
> > >
> > > Steven Liu
> > >
> > > On Tue, 2009-12-29 at 12:04 +0200, Michael S. Tsirkin wrote:
> > > > Most likely DISPLAY not set.
> > > >
> > > > On Sat, Dec 26, 2009 at 07:40:01AM -0300, Dejun.Liu wrote:
> > > > > Hi,
> > > > >
> > > > > Im a newbie to qemu use.
> > > > > so i got a lot error!..
> > > > >
> > > > > I build qemu from source with the version 0.10.3,
> > > > >
> > > > > But when i start qemu with the command below, i got the message
> > > > > Could not initialize SDL - exiting
> > > > >
> > > > > after that Qemu exit. I dig the web ,but i could not find any
> > > > > answers,so
> > > > > I ask this question in this maillist,sorry for this.
> > > > >
> > > > > Command:
> > > > > /angstrom-dev/staging/i686-linux/usr/bin/qemu-system-arm -M
> > > > > versatilepb -usb -usbdevice wacom-tablet -show-cursor -m 64
> > > > > -kernel
> > > > > /home/protocol/study/OE/angstrom-dev/deploy/glibc/images/qemuarm/zImage-2.6.25+2.6.26-rc4-r6-qemuarm.bin
> > > > > -drive
> > > > > file=/home/protocol/study/OE/angstrom-dev/deploy/glibc/images/qemuarm/Angstrom-x11-image-glibc-ipk-2009.X-stable-qemuarm.rootfs.ext3
> > > > > -append "root=/dev/sda"
> > > > >
> > > > > And ldd result of qemu-system-arm is:
> > > > >
> > > > > r...@dejunliu-desktop:~# ldd `which qemu-system-arm`
> > > > > linux-gate.so.1 => (0xb7f4c000)
> > > > > libm.so.6 => /lib/tls/i686/cmov/libm.so.6 (0xb7f18000)
> > > > > libz.so.1
> > > > > =>
> > > > > /home/protocol/study/OE/angstrom-dev/staging/i686-linux/usr/lib/libz.so.1
> > > > > (0xb7f06000)
> > > > > libpthread.so.0 => /lib/tls/i686/cmov/libpthread.so.0
> > > > > (0xb7eed000)
> > > > > librt.so.1 => /lib/tls/i686/cmov/librt.so.1 (0xb7ee4000)
> > > > > libutil.so.1 => /lib/tls/i686/cmov/libutil.so.1 (0xb7ee)
> > > > > libSDL-1.2.so.0
> > > > > =>
> > > > > /home/protocol/study/OE/angstrom-dev/staging/i686-linux/usr/lib/libSDL-1.2.so.0
> > > > > (0xb7e9f000)
> > > > > libncurses.so.5
> > > > > =>
> > > > > /home/protocol/study/OE/angstrom-dev/staging/i686-linux/usr/lib/libncurses.so.5
> > > > > (0xb7e5c000)
> > > > > libc.so.6 => /lib/tls/i686/cmov/libc.so.6 (0xb7d0c000)
> > > > > /lib/ld-linux.so.2 (0xb7f4d000)
> > > > > libdl.so.2 => /lib/tls/i686/cmov/libdl.so.2 (0xb7d08000)
Oh, X is not linked to. If this is intentional, run with -curses flag,
if not recompile qemu after installing X development libraries.
You should be able to compile the following program without errors:
#include
#if defined(SDL_VIDEO_DRIVER_X11)
#include
#else
#error No x11 support
#endif
int main(void) { return 0; }
You can also try applying the following patch, it might give
you more info.
> > > > >
> > > > > would anyone like to help me to solve this problem?
> > > > >
> > > > >
> > > > > Cheers
> > > > > Steven Liu
> > > > >
> > > > >
> > > > > ---
> > > > > Confidentiality Notice: The information contained in this e-mail and
> > > > > any accompanying attachment(s)
> > > > > is intended only for the use of the intended recipient and may be
> > > > > confidential and/or privileged of
> > > > > Neusoft Corporation, its subsidiaries and/or its affiliates. If any
> > > > > reader of this communication is
> > > > > not the intended recipient, unauthorized use, forwarding, printing,
> > > > > storing, disclosure or copying
> > > > > is strictly prohibited, and may be unlawful.If you have received this
> > > > > communication in error,please
> > > > > immediately notify the sender by return e-mail, and delete the
> > > > > original message and all copies from
> > > > > your system. Thank you.
> > > > >
[Qemu-devel] Re: commit rules for common git tree
On Tue, Dec 29, 2009 at 09:03:18PM +0100, Aurelien Jarno wrote: > On Tue, Dec 29, 2009 at 08:12:57PM +0200, Michael S. Tsirkin wrote: > > On Tue, Dec 29, 2009 at 06:40:31PM +0100, Aurelien Jarno wrote: > > > On Tue, Dec 29, 2009 at 07:23:28PM +0200, Michael S. Tsirkin wrote: > > > > On Sun, Dec 27, 2009 at 05:01:38PM -0600, Anthony Liguori wrote: > > > > > Likewise, if you see a patch go in that you think would have > > > > > benefited > > > > > from being on the list, point it out. > > > > > > > > How *would* I see it? I guess I could write scripts that correlated git > > > > logs (or qemu commit list if it is ever resurrected) with mailing list > > > > archive. If patch is not there, look in mailing list around the time > > > > for a hint: could have explanation in the discussion. Or maybe it is > > > > part of a series someone pushed ... you get the point. > > > > > > > > > > All those commit have a single Signed-of-by: line. That decreases the > > > number of patches to take. > > > > > > Anyway I still don't really understand you are trying to achieve here, > > > and without pointing to real problems, I am not sure we are going to > > > understand. > > > > What would you call real problems? > > - You listed complains 3 times as an issue for committers. > > Is this a problem worth solving? I think my suggestions > > will help solve it. > > This has nothing to do with "all patches should be submitted to the > mailing list first" Let's start with "should be submitted to the mailing list" without "first". Makes sense? > > - Even codewise: it's not uncommon for commit X to revert commit Y by > > another person, at least partially. I can point them out > > privately, prefer not to do it publically. This hurts git bisect and > > generally makes following history harder. Unavoidable but seems less > > common with reviewed patches. > > Then those are the real problem, and should be avoided. Everyone did this at some point. It's human to err. > Pointing the > people that they should not commit patches like that is the way to go > IMHO. Yes but one does not *know* one is making a mistake beforehand. So I think "like that" is "without giving anyone a chance to review". > > > The life of committers is not really easy, and the patch submitters do > > > not really help to improve it: > > > - a lot of patches are only reviewed by the committers, not so many > > > persons send Acked-by: mails. > > > - you have to deal with code you don't understand > > > - you have to track continuously respined series (with patches moving > > > from one to another, being merged or changing title) > > > - you get complains when you review a patch > > > - you get complains when you don't review a patch fast enough > > > - you get complains after you applied a patch > > > - etc... > > > > Most complaints are result of somewhat opaque nature of the commit > > process. For example, people would complain "where's my patch" often to > > be said: it is applied, not yet pushed. Sending mail "applied thanks" > > would solve this. I am suggesting making it more transparent and reduce > > number of complaints. Thus life of committers will become easier :). > > > > I don't get this kind of complains very often. > > I get complains because: > - I don't review patches fast enough. Here other people can help by > reviewing the patches and sending an Acked-by: > - I have applied a patch that breaks something. It get even worse when > the patch has been committed sometimes ago and the patch submitter has > disappear. Again other people can help reviewing patches to catch > common mistakes. > > If others make efforts sharing the load a bit, I am fine making efforts. > Otherwise no. Well, commits without post are not making sharing the load easy: - If patches are applied without notifying the list others waste time reviewing a patch that was already reviewed and applied. - If code changes without notifying the list, others must consider and merge multiple channels when trying to understand a patch (e.g. it might not even apply to your tree, you must pull to understand it). -- MST
Re: [Qemu-devel] [PATCH] add "info ioapic" monitor command
On Tue, Dec 29, 2009 at 06:49:14PM -0200, Luiz Capitulino wrote:
> On Tue, 29 Dec 2009 20:49:53 +0200
> Gleb Natapov wrote:
>
> > On Tue, Dec 29, 2009 at 03:39:29PM -0200, Luiz Capitulino wrote:
> > > On Tue, 29 Dec 2009 18:53:44 +0200
> > > Gleb Natapov wrote:
> > >
> > > > On Tue, Dec 29, 2009 at 02:37:20PM -0200, Luiz Capitulino wrote:
> > > > > On Thu, 24 Dec 2009 17:02:42 +0200
> > > > > Gleb Natapov wrote:
> > > > >
> > > > > > +
> > > > > > +static const char *delivery_mode_string[] = {"fixed", "lowprio",
> > > > > > "smi", "res",
> > > > > > + "nmi", "init", "res",
> > > > > > "extint"};
> > > > > > +
> > > > > > +void do_info_ioapic(Monitor *mon)
> > > > > > +{
> > > > > > +int i;
> > > > > > +
> > > > > > +if (!ioapic)
> > > > > > +return;
> > > > > > +for (i = 0; i < IOAPIC_NUM_PINS; i++) {
> > > > > > +uint64 e = ioapic->ioredtbl[i];
> > > > > > +monitor_printf(mon, "%2d: ", i);
> > > > > > +if (e & IOAPIC_LVT_MASKED) {
> > > > > > +monitor_printf(mon, "masked\n");
> > > > > > +} else {
> > > > > > +uint8_t vec = e & 0xff;
> > > > > > +uint8_t trig_mode = ((e >> 15) & 1);
> > > > > > +uint8_t dest = e >> 56;
> > > > > > +uint8_t dest_mode = (e >> 11) & 1;
> > > > > > +uint8_t delivery_mode = (e >> 8) & 7;
> > > > > > +uint8_t polarity = (e >> 13) & 1;
> > > > > > +monitor_printf(mon, "vec=%3d %s %s acive-%s %s
> > > > > > dest=%d\n",
> > > > > > + vec,
> > > > > > + delivery_mode_string[delivery_mode],
> > > > > > + dest_mode ? "logical":"physical",
> > > > > > + polarity ? "low" : "high",
> > > > > > + trig_mode ? "level": "edge",
> > > > > > + dest);
> > > > > > +}
> > > > > > +}
> > > > > > +}
> > > > >
> > > > > New Monitor handlers should return QObjects, monitor_printf()
> > > > > can only be used in the print handler.
> > > > >
> > > > So I want to add only print handler. This is debug info only, no
> > > > management should ever care about it.
> > >
> > > Well, this is possible (at least for now) but there are plans to
> > > have an external shell.
> > >
> > > Also, I don't want people to avoid writing handlers because
> > > they feel it's difficult.
> > >
> > > > > You can take a look at qemu_chr_info() for an example, as it does
> > > > > what I think you should do here: build a qdict for each pin and
> > > > > return a qlist or return another qdict if it makes sense (or will
> > > > > make in the future) to have more the one ioapic.
> > > > I don't understand a single word from what you are saying :(, but
> > > > qemu_chr_info() looks scary. Actually I don't understand what it does at
> > > > all.
> > >
> > > Ouch. :((
> > >
> > Well, after looking at it for 10 more minutes I can tell that it build
> > some kind of object hierarchy, but it is too low level and requires from
> > casual command writer too deep knowledge of the infrastructure.
>
> "too deep" is too strong :) a simple text explaining how to use the
> API would be enough, imho.
>
May be. May be adding helpers functions to handle common cases would be
helpful too. For instance iterating over some data structure while creating
qdict object for each one of them and collecting all of them in one list
is used all over the palace as far as I can see.
> > > > > I'm still thinking in ways to make the work of writing the new
> > > > > Monitor handlers easier..
> > > > Something more simple is definitely needed. If I need to pars the data
> > > > structure to some intermediate language and then parse it back again
> > > > just to add
> > > > debug command I will not event consider adding it. One more tracepoint
> > > > in the kernel will take 10min to add and will accomplish the same goal
> > > > to me albeit in less elegant way.
> > >
> > > Actually, you're building objects and iterating over them to get them
> > > printed, it's not parsing and it's common to do that in high level
> > > languages.
> > I high level language the syntax hides all the complexity.
>
> Right, but the handling of objects is still done by the programmer.
>
The handling of objects is very different when it is not part of the
language.
There is a difference between this:
static void qemu_chr_qlist_iter(QObject *obj, void *opaque)
{
QDict *chr_dict;
Monitor *mon = opaque;
chr_dict = qobject_to_qdict(obj);
monitor_printf(mon, "%s: filename=%s\n", qdict_get_str(chr_dict, "label"),
qdict_get_str(chr_dict, "filename"));
}
void qemu_chr_info_print(Monitor *mon, const QObject *ret_data)
{
qlist_iter(qobject_to_qlist(ret_data), qemu_chr_qlist_iter, mon);
}
And this:
for i in chardev:
print "%(label)s: filename=
[Qemu-devel] [PATCHv2] add "info ioapic" monitor command
Knowing ioapic configuration is very useful for the poor soles
how need to debug guest occasionally.
Signed-off-by: Gleb Natapov
--
I am starring to learn this QObject kung-fu. One question:
Why qlist_iter(..., func, ...) and not
FOREACH_QOBJ() {
do things
}
diff --git a/hw/ioapic.c b/hw/ioapic.c
index b0ad78f..efb9744 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -24,6 +24,12 @@
#include "pc.h"
#include "qemu-timer.h"
#include "host-utils.h"
+#include "monitor.h"
+#include "qint.h"
+#include "qlist.h"
+#include "qdict.h"
+#include "qstring.h"
+#include "qjson.h"
//#define DEBUG_IOAPIC
@@ -50,6 +56,8 @@ struct IOAPICState {
uint64_t ioredtbl[IOAPIC_NUM_PINS];
};
+static struct IOAPICState *ioapic;
+
static void ioapic_service(IOAPICState *s)
{
uint8_t i;
@@ -232,7 +240,7 @@ qemu_irq *ioapic_init(void)
qemu_irq *irq;
int io_memory;
-s = qemu_mallocz(sizeof(IOAPICState));
+ioapic = s = qemu_mallocz(sizeof(IOAPICState));
ioapic_reset(s);
io_memory = cpu_register_io_memory(ioapic_mem_read,
@@ -245,3 +253,70 @@ qemu_irq *ioapic_init(void)
return irq;
}
+
+static void qemu_ioapic_qlist_iter(QObject *data, void *opaque)
+{
+QDict *qdict = qobject_to_qdict(data);
+Monitor *mon = opaque;
+
+monitor_printf(mon, "%2"PRId64": ", qdict_get_int(qdict, "index"));
+if (qdict_haskey(qdict, "masked")) {
+monitor_printf(mon, "masked\n");
+} else {
+monitor_printf(mon, "vec=%3"PRId64" %s %s acive-%s %s
dest=%"PRId64"\n",
+ qdict_get_int(qdict, "vector"),
+ qdict_get_str(qdict, "delivery_mode"),
+ qdict_get_str(qdict, "dest_mode"),
+ qdict_get_str(qdict, "polarity"),
+ qdict_get_str(qdict, "trig_mode"),
+ qdict_get_int(qdict, "destination"));
+}
+}
+
+void monitor_print_ioapic(Monitor *mon, const QObject *ret_data)
+{
+qlist_iter(qobject_to_qlist(ret_data), qemu_ioapic_qlist_iter, mon);
+}
+
+static const char *delivery_mode_string[] = {"fixed", "lowprio", "smi", "res",
+ "nmi", "init", "res", "extint"};
+
+void do_info_ioapic(Monitor *mon, QObject **ret_data)
+{
+int i;
+QList *list;
+
+*ret_data = NULL;
+
+if (!ioapic)
+return;
+
+list = qlist_new();
+
+for (i = 0; i < IOAPIC_NUM_PINS; i++) {
+QObject *obj;
+uint64 e = ioapic->ioredtbl[i];
+if (e & IOAPIC_LVT_MASKED) {
+obj = qobject_from_jsonf("{'index': %d, 'masked': 1}", i);
+} else {
+uint8_t vec = e & 0xff;
+uint8_t trig_mode = ((e >> 15) & 1);
+uint8_t dest = e >> 56;
+uint8_t dest_mode = (e >> 11) & 1;
+uint8_t delivery_mode = (e >> 8) & 7;
+uint8_t polarity = (e >> 13) & 1;
+obj = qobject_from_jsonf("{'index': %d, 'vector': %d,"
+ "'delivery_mode': %s, 'dest_mode': %s,"
+ "'polarity': %s, 'trig_mode': %s,"
+ "'destination': %d}",
+ i, vec,
+ delivery_mode_string[delivery_mode],
+ dest_mode ? "logical":"physical",
+ polarity ? "low" : "high",
+ trig_mode ? "level": "edge",
+ dest);
+}
+qlist_append_obj(list, obj);
+}
+*ret_data = QOBJECT(list);
+}
diff --git a/hw/pc.h b/hw/pc.h
index 03ffc91..3e39444 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -2,6 +2,7 @@
#define HW_PC_H
#include "qemu-common.h"
+#include "qobject.h"
/* PC-style peripherals (also used by other machines). */
@@ -45,6 +46,8 @@ int apic_accept_pic_intr(CPUState *env);
void apic_deliver_pic_intr(CPUState *env, int level);
int apic_get_interrupt(CPUState *env);
qemu_irq *ioapic_init(void);
+void do_info_ioapic(Monitor *mon, QObject **ret_data);
+void monitor_print_ioapic(Monitor *mon, const QObject *data);
void ioapic_set_irq(void *opaque, int vector, int level);
void apic_reset_irq_delivered(void);
int apic_get_irq_delivered(void);
diff --git a/monitor.c b/monitor.c
index c0dc48e..367e330 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2625,6 +2625,14 @@ static const mon_cmd_t info_cmds[] = {
.mhandler.info = do_info_roms,
},
{
+.name = "ioapic",
+.args_type = "",
+.params = "",
+.help = "show ioapic config",
+.user_print = monitor_print_ioapic,
+.mhandler.info_new = do_info_ioapic,
+},
+{
.name = NULL,
},
};
--
Gleb.
Re: [Qemu-devel] [PATCHv2] add "info ioapic" monitor command
On Wed, Dec 30, 2009 at 11:50 AM, Gleb Natapov wrote: > Knowing ioapic configuration is very useful for the poor soles > how need to debug guest occasionally. > +static struct IOAPICState *ioapic; Ugly. I really think the monitor interface needs to be changed to take an opaque parameter. The parameter can passed to the caller with for example a registration function, like the patches I sent before QObject conversion. > +void monitor_print_ioapic(Monitor *mon, const QObject *ret_data) In this case the function would be void monitor_print_ioapic(Monitor *mon, const QObject *ret_data, void *opaque) where opaque replaces the static ioapic variable. > +void do_info_ioapic(Monitor *mon, QObject **ret_data) Same here.
Re: [Qemu-devel] [PATCHv2] add "info ioapic" monitor command
On Wed, Dec 30, 2009 at 12:01:28PM +, Blue Swirl wrote: > On Wed, Dec 30, 2009 at 11:50 AM, Gleb Natapov wrote: > > Knowing ioapic configuration is very useful for the poor soles > > how need to debug guest occasionally. > > > +static struct IOAPICState *ioapic; > > Ugly. I really think the monitor interface needs to be changed to take > an opaque parameter. > Agree, but that's the reality now. Pic does the same. As long as there is only one IOAPIC (and there is no indication that there will be more) that is OK. > The parameter can passed to the caller with for example a registration > function, like the patches I sent before QObject conversion. > > > +void monitor_print_ioapic(Monitor *mon, const QObject *ret_data) > > In this case the function would be > void monitor_print_ioapic(Monitor *mon, const QObject *ret_data, void *opaque) > where opaque replaces the static ioapic variable. > > > +void do_info_ioapic(Monitor *mon, QObject **ret_data) > > Same here. > -- Gleb.
Re: [Qemu-devel] [PATCHv2] add "info ioapic" monitor command
On Wed, 30 Dec 2009 14:05:10 +0200 Gleb Natapov wrote: > On Wed, Dec 30, 2009 at 12:01:28PM +, Blue Swirl wrote: > > On Wed, Dec 30, 2009 at 11:50 AM, Gleb Natapov wrote: > > > Knowing ioapic configuration is very useful for the poor soles > > > how need to debug guest occasionally. > > > > > +static struct IOAPICState *ioapic; > > > > Ugly. > > > Agree And now it seems like even you dont agree with me!!!
Re: [Qemu-devel] [PATCHv2] add "info ioapic" monitor command
On Wed, Dec 30, 2009 at 02:08:27PM +0200, Izik Eidus wrote: > On Wed, 30 Dec 2009 14:05:10 +0200 > Gleb Natapov wrote: > > > On Wed, Dec 30, 2009 at 12:01:28PM +, Blue Swirl wrote: > > > On Wed, Dec 30, 2009 at 11:50 AM, Gleb Natapov wrote: > > > > Knowing ioapic configuration is very useful for the poor soles > > > > how need to debug guest occasionally. > > > > > > > +static struct IOAPICState *ioapic; > > > > > > Ugly. > > > > > Agree > > And now it seems like even you dont agree with me!!! > I agree, but all other info functions in qemu do exactly same: referencing global data. This is ugly _and_ this nothing to do with my patch. -- Gleb.
Re: [Qemu-devel] "Hot" resizing QCow2 image using a simple python script
Laurent Coustet schrieb: > I've seen the code on git a little bit, seems rather simple to me, but > what do you think is the better way to integrate such a fonctionality > ? New command ? > > The best approch for me I think is to extend "convert". > > Example: > qemu-img -f qcow2 -O qcow2 original5G.qcow2 output10G.qcow2 +5G > > Thanks, Extending convert would be good if you want to create a new image. This syntax might be better (no new parameter, final size instead of delta): qemu-img convert -f qcow2 -O qcow2 -o size=10G original5G.qcow2 output10G.qcow2 For resizing an existing image, a new command is needed: qemu-img resize -f qcow2 -o size=10G original5G.qcow2 Although there is no urgent need for either variant... Regards Stefan
Re: [Qemu-devel] [PATCH] add "info ioapic" monitor command
On Wed, 30 Dec 2009 12:45:08 +0200
Gleb Natapov wrote:
> On Tue, Dec 29, 2009 at 06:49:14PM -0200, Luiz Capitulino wrote:
> > On Tue, 29 Dec 2009 20:49:53 +0200
> > Gleb Natapov wrote:
> >
> > > On Tue, Dec 29, 2009 at 03:39:29PM -0200, Luiz Capitulino wrote:
> > > > On Tue, 29 Dec 2009 18:53:44 +0200
> > > > Gleb Natapov wrote:
> > > >
> > > > > On Tue, Dec 29, 2009 at 02:37:20PM -0200, Luiz Capitulino wrote:
> > > > > > On Thu, 24 Dec 2009 17:02:42 +0200
> > > > > > Gleb Natapov wrote:
> > > > > >
> > > > > > > +
> > > > > > > +static const char *delivery_mode_string[] = {"fixed", "lowprio",
> > > > > > > "smi", "res",
> > > > > > > + "nmi", "init",
> > > > > > > "res", "extint"};
> > > > > > > +
> > > > > > > +void do_info_ioapic(Monitor *mon)
> > > > > > > +{
> > > > > > > +int i;
> > > > > > > +
> > > > > > > +if (!ioapic)
> > > > > > > +return;
> > > > > > > +for (i = 0; i < IOAPIC_NUM_PINS; i++) {
> > > > > > > +uint64 e = ioapic->ioredtbl[i];
> > > > > > > +monitor_printf(mon, "%2d: ", i);
> > > > > > > +if (e & IOAPIC_LVT_MASKED) {
> > > > > > > +monitor_printf(mon, "masked\n");
> > > > > > > +} else {
> > > > > > > +uint8_t vec = e & 0xff;
> > > > > > > +uint8_t trig_mode = ((e >> 15) & 1);
> > > > > > > +uint8_t dest = e >> 56;
> > > > > > > +uint8_t dest_mode = (e >> 11) & 1;
> > > > > > > +uint8_t delivery_mode = (e >> 8) & 7;
> > > > > > > +uint8_t polarity = (e >> 13) & 1;
> > > > > > > +monitor_printf(mon, "vec=%3d %s %s acive-%s %s
> > > > > > > dest=%d\n",
> > > > > > > + vec,
> > > > > > > + delivery_mode_string[delivery_mode],
> > > > > > > + dest_mode ? "logical":"physical",
> > > > > > > + polarity ? "low" : "high",
> > > > > > > + trig_mode ? "level": "edge",
> > > > > > > + dest);
> > > > > > > +}
> > > > > > > +}
> > > > > > > +}
> > > > > >
> > > > > > New Monitor handlers should return QObjects, monitor_printf()
> > > > > > can only be used in the print handler.
> > > > > >
> > > > > So I want to add only print handler. This is debug info only, no
> > > > > management should ever care about it.
> > > >
> > > > Well, this is possible (at least for now) but there are plans to
> > > > have an external shell.
> > > >
> > > > Also, I don't want people to avoid writing handlers because
> > > > they feel it's difficult.
> > > >
> > > > > > You can take a look at qemu_chr_info() for an example, as it does
> > > > > > what I think you should do here: build a qdict for each pin and
> > > > > > return a qlist or return another qdict if it makes sense (or will
> > > > > > make in the future) to have more the one ioapic.
> > > > > I don't understand a single word from what you are saying :(, but
> > > > > qemu_chr_info() looks scary. Actually I don't understand what it does
> > > > > at
> > > > > all.
> > > >
> > > > Ouch. :((
> > > >
> > > Well, after looking at it for 10 more minutes I can tell that it build
> > > some kind of object hierarchy, but it is too low level and requires from
> > > casual command writer too deep knowledge of the infrastructure.
> >
> > "too deep" is too strong :) a simple text explaining how to use the
> > API would be enough, imho.
> >
> May be. May be adding helpers functions to handle common cases would be
> helpful too.
Sure.
> For instance iterating over some data structure while creating
> qdict object for each one of them and collecting all of them in one list
> is used all over the palace as far as I can see.
The problem I have in devising a helper like that is how to do
the mapping between data structure member and dict key, given that they
have to be accessed at run time.
Also, I think that even some simple data structures may need human
intervention, when nested dicts are used for example.
But maybe, the helper could accept an array with that information, that
could be filled by the user. But I wonder if that really makes things
simple or we could make this as part of the structure itself someway.
Ideas?
> > > > > > I'm still thinking in ways to make the work of writing the new
> > > > > > Monitor handlers easier..
> > > > > Something more simple is definitely needed. If I need to pars the data
> > > > > structure to some intermediate language and then parse it back again
> > > > > just to add
> > > > > debug command I will not event consider adding it. One more tracepoint
> > > > > in the kernel will take 10min to add and will accomplish the same goal
> > > > > to me albeit in less elegant way.
> > > >
> > > > Actually, you're building objects and iterating over them to get them
> > > > printed, it's not parsing and it's common to do that in high level
>
[Qemu-devel] Re: commit rules for common git tree
On Sun, Dec 27, 2009 at 05:01:38PM -0600, Anthony Liguori wrote: > I think it's a good idea to use the mailing list whenever possible. > Likewise, if you see a patch go in that you think would have benefited > from being on the list, point it out. Sometimes it would have benefited *others* if the patch was posted (either before or after commit). Consider commit: commit 3caf2562c21278aa9992bc7e6b5bbe98bc79f92e Author: malc Date: Wed Dec 30 04:26:34 2009 +0300 sdl: print the reason why SDL thinks SDL_Init failed before exiting Signed-off-tby: malc So after this is applied, we print out failure reason which is good. But this was only applied on master, and I think this patch would be good to have on 0.12 and even 0.11 branches: it is very safe, and SDL init failures are a FAQ. I hope this illustrates how it is good to post even obvious patches to the list. --
[Qemu-devel] Re: [PATCHv2] add "info ioapic" monitor command
On Wed, 30 Dec 2009 13:50:43 +0200
Gleb Natapov wrote:
> I am starring to learn this QObject kung-fu.
Nice, you really got how to do it. Just two minor comments.
> One question:
> Why qlist_iter(..., func, ...) and not
> FOREACH_QOBJ() {
>do things
> }
Well, when I started working on the QObjects I was still getting
familiar with QEMU internals and just ignored the FOREACH_ loops.
Later, I realized that they could be better but then we were
planning to have libqmp and now I'm wondering if it's ok to expose
data structure members to the public.
> diff --git a/hw/ioapic.c b/hw/ioapic.c
> index b0ad78f..efb9744 100644
> --- a/hw/ioapic.c
> +++ b/hw/ioapic.c
> @@ -24,6 +24,12 @@
> #include "pc.h"
> #include "qemu-timer.h"
> #include "host-utils.h"
> +#include "monitor.h"
> +#include "qint.h"
> +#include "qlist.h"
> +#include "qdict.h"
> +#include "qstring.h"
> +#include "qjson.h"
You can include qemu-objects.h.
> +void do_info_ioapic(Monitor *mon, QObject **ret_data)
> +{
> +int i;
> +QList *list;
> +
> +*ret_data = NULL;
> +
> +if (!ioapic)
> +return;
> +
> +list = qlist_new();
> +
> +for (i = 0; i < IOAPIC_NUM_PINS; i++) {
> +QObject *obj;
> +uint64 e = ioapic->ioredtbl[i];
> +if (e & IOAPIC_LVT_MASKED) {
> +obj = qobject_from_jsonf("{'index': %d, 'masked': 1}", i);
'masked' should be a bool, using %i will do it, like:
obj = qobject_from_jsonf("{'index': %d, 'masked': %i}", i);
[Qemu-devel] Re: [PATCHv2] add "info ioapic" monitor command
On Wed, Dec 30, 2009 at 11:19:30AM -0200, Luiz Capitulino wrote:
> On Wed, 30 Dec 2009 13:50:43 +0200
> Gleb Natapov wrote:
>
> > I am starring to learn this QObject kung-fu.
>
> Nice, you really got how to do it. Just two minor comments.
>
> > One question:
> > Why qlist_iter(..., func, ...) and not
> > FOREACH_QOBJ() {
> >do things
> > }
>
> Well, when I started working on the QObjects I was still getting
> familiar with QEMU internals and just ignored the FOREACH_ loops.
>
> Later, I realized that they could be better but then we were
> planning to have libqmp and now I'm wondering if it's ok to expose
> data structure members to the public.
>
> > diff --git a/hw/ioapic.c b/hw/ioapic.c
> > index b0ad78f..efb9744 100644
> > --- a/hw/ioapic.c
> > +++ b/hw/ioapic.c
> > @@ -24,6 +24,12 @@
> > #include "pc.h"
> > #include "qemu-timer.h"
> > #include "host-utils.h"
> > +#include "monitor.h"
> > +#include "qint.h"
> > +#include "qlist.h"
> > +#include "qdict.h"
> > +#include "qstring.h"
> > +#include "qjson.h"
>
> You can include qemu-objects.h.
>
Hm. Copied all this from monitor.c
> > +void do_info_ioapic(Monitor *mon, QObject **ret_data)
> > +{
> > +int i;
> > +QList *list;
> > +
> > +*ret_data = NULL;
> > +
> > +if (!ioapic)
> > +return;
> > +
> > +list = qlist_new();
> > +
> > +for (i = 0; i < IOAPIC_NUM_PINS; i++) {
> > +QObject *obj;
> > +uint64 e = ioapic->ioredtbl[i];
> > +if (e & IOAPIC_LVT_MASKED) {
> > +obj = qobject_from_jsonf("{'index': %d, 'masked': 1}", i);
>
> 'masked' should be a bool, using %i will do it, like:
>
>
> obj = qobject_from_jsonf("{'index': %d, 'masked': %i}", i);
No need to put anything here? --^?
OR should it be
obj = qobject_from_jsonf("{'index': %d, 'masked': %i}", i, true);
--
Gleb.
Re: [Qemu-devel] [PATCHv2] add "info ioapic" monitor command
On Wed, 30 Dec 2009 12:01:28 + Blue Swirl wrote: > On Wed, Dec 30, 2009 at 11:50 AM, Gleb Natapov wrote: > > Knowing ioapic configuration is very useful for the poor soles > > how need to debug guest occasionally. > > > +static struct IOAPICState *ioapic; > > Ugly. I really think the monitor interface needs to be changed to take > an opaque parameter. > > The parameter can passed to the caller with for example a registration > function, like the patches I sent before QObject conversion. Do you mind updating your series and sending it again? You could make the opaque parameter part of info_new or cmd_new, provided the new handlers have global data so that we can see better the benefit in practice.
[Qemu-devel] Re: [PATCHv2] add "info ioapic" monitor command
On Wed, 30 Dec 2009 15:22:19 +0200
Gleb Natapov wrote:
> > > +void do_info_ioapic(Monitor *mon, QObject **ret_data)
> > > +{
> > > +int i;
> > > +QList *list;
> > > +
> > > +*ret_data = NULL;
> > > +
> > > +if (!ioapic)
> > > +return;
> > > +
> > > +list = qlist_new();
> > > +
> > > +for (i = 0; i < IOAPIC_NUM_PINS; i++) {
> > > +QObject *obj;
> > > +uint64 e = ioapic->ioredtbl[i];
> > > +if (e & IOAPIC_LVT_MASKED) {
> > > +obj = qobject_from_jsonf("{'index': %d, 'masked': 1}", i);
> >
> > 'masked' should be a bool, using %i will do it, like:
> >
> >
> > obj = qobject_from_jsonf("{'index': %d, 'masked': %i}", i);
> No need to put anything here? --^?
> OR should it be
> obj = qobject_from_jsonf("{'index': %d, 'masked': %i}", i, true);
Oh yes, you right. Actually, you can do:
obj = qobject_from_jsonf("{'index': %d, 'masked': true }", i);
I would also include this key in the unmasked dict (the one
you fill in the else clause), just for consistency in the protocol.
And yes, having to worry about the protocol in the handler is a sign
that we have to improve.
Re: [Qemu-devel] [PATCH] add "info ioapic" monitor command
On 12/30/2009 03:27 AM, Gleb Natapov wrote: On Tue, Dec 29, 2009 at 08:00:09PM -0600, Anthony Liguori wrote: On 12/24/2009 09:02 AM, Gleb Natapov wrote: Knowing ioapic configuration is very useful for the poor soles how need to debug guest occasionally. Can this be implemented in terms of VMState? What do you mean and why it whould be any better? We've been discussing having a command that's something like 'info-device ioapic' that would dump the device state based on what's stored in VMState for the device. The advantage of a command like this is that it works for any qdev device making it much more useful. The disadvantage is that at the moment, we store ioredtbl as an array of uint64s. To get the level of info you're looking for we would need to teach VMState how to decode this structure. Regards, Anthony Liguori -- Gleb.
Re: [Qemu-devel] [PATCH] add "info ioapic" monitor command
On 12/30/2009 12:53 AM, Avi Kivity wrote: On 12/30/2009 04:00 AM, Anthony Liguori wrote: On 12/24/2009 09:02 AM, Gleb Natapov wrote: Knowing ioapic configuration is very useful for the poor soles how need to debug guest occasionally. Can this be implemented in terms of VMState? That's a good idea, but it would mean that we'd need into ioapic and info ioapic-kvm if we go for a split implementation. I don't think that's a problem. A developer will be smart enough to know that they need to do that (and will get an error if they don't). Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH] add "info ioapic" monitor command
On 12/30/2009 04:17 PM, Anthony Liguori wrote: That's a good idea, but it would mean that we'd need into ioapic and info ioapic-kvm if we go for a split implementation. I don't think that's a problem. A developer will be smart enough to know that they need to do that (and will get an error if they don't). I wasn't worried about that, only the increased code duplication. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] add "info ioapic" monitor command
On Wed, Dec 30, 2009 at 08:16:30AM -0600, Anthony Liguori wrote: > On 12/30/2009 03:27 AM, Gleb Natapov wrote: > >On Tue, Dec 29, 2009 at 08:00:09PM -0600, Anthony Liguori wrote: > >>On 12/24/2009 09:02 AM, Gleb Natapov wrote: > >>>Knowing ioapic configuration is very useful for the poor soles > >>>how need to debug guest occasionally. > >> > >>Can this be implemented in terms of VMState? > >> > >What do you mean and why it whould be any better? > > We've been discussing having a command that's something like > 'info-device ioapic' that would dump the device state based on > what's stored in VMState for the device. > > The advantage of a command like this is that it works for any qdev > device making it much more useful. > > The disadvantage is that at the moment, we store ioredtbl as an > array of uint64s. To get the level of info you're looking for we > would need to teach VMState how to decode this structure. > So this is completely orthogonal to my patch. There are devices already that have info command. No need to hold the patch just because sometime in the feature VMState may be extended. Other then that adding print function to VMState sounds like a good idea. -- Gleb.
Re: [Qemu-devel] [PATCH] add "info ioapic" monitor command
On Wed, Dec 30, 2009 at 04:26:11PM +0200, Gleb Natapov wrote: > On Wed, Dec 30, 2009 at 08:16:30AM -0600, Anthony Liguori wrote: > > On 12/30/2009 03:27 AM, Gleb Natapov wrote: > > >On Tue, Dec 29, 2009 at 08:00:09PM -0600, Anthony Liguori wrote: > > >>On 12/24/2009 09:02 AM, Gleb Natapov wrote: > > >>>Knowing ioapic configuration is very useful for the poor soles > > >>>how need to debug guest occasionally. > > >> > > >>Can this be implemented in terms of VMState? > > >> > > >What do you mean and why it whould be any better? > > > > We've been discussing having a command that's something like > > 'info-device ioapic' that would dump the device state based on > > what's stored in VMState for the device. > > > > The advantage of a command like this is that it works for any qdev > > device making it much more useful. > > > > The disadvantage is that at the moment, we store ioredtbl as an > > array of uint64s. To get the level of info you're looking for we > > would need to teach VMState how to decode this structure. > > > > So this is completely orthogonal to my patch. There are devices already > that have info command. No need to hold the patch just because sometime > in the feature VMState may be extended. Other then that adding print > function to VMState sounds like a good idea. BTW I'd like this to be backported to 0.12 too. -- Gleb.
Re: [Qemu-devel] [Bug] qemu-system-ppc: "invalid/unsupported opcode" during debug session
Am 29.12.2009 um 22:07 schrieb Aurelien Jarno : On Tue, Dec 29, 2009 at 04:09:17PM +0100, Stefan Weil wrote: Test environment: * ppc-softmmu/qemu-system-ppc running on x86_64 host * emulated ppc is running debian lenny While debugging on the emulated ppc (each time when a shared library is loaded after "r" command?), qemu-system-ppc prints this error message: invalid/unsupported opcode: 00 - 00 - 00 () 4800fa44 1 If logging is enabled, the error message goes to qemu.log: IN: 0xc0013488: nop 0xc001348c: rlwinm r3,r3,0,0,19 0xc0013490: li r4,128 0xc0013494: mtctr r4 0xc0013498: mr r6,r3 0xc001349c: dcbst r0,r3 invalid/unsupported opcode: 00 - 00 - 00 () 4800fa44 1 IN: 0x4800fa40: twger2,r2 0x4800fa44: .long 0x0 The problem is that QEMU doesn't stop the decoding of instructions when it encounters a trap instruction. We should probably either end the TB in that case, or avoid printing "invalid/unsupported opcode", as this instruction will actually never been executed. Given how seldom they occur, it's probably best (easiest to read) to end the TB. Alex
Re: [Qemu-devel] [PATCHv2] add "info ioapic" monitor command
On 12/30/2009 01:50 PM, Gleb Natapov wrote:
Knowing ioapic configuration is very useful for the poor soles
how need to debug guest occasionally.
+
+void do_info_ioapic(Monitor *mon, QObject **ret_data)
+{
+int i;
+QList *list;
+
+*ret_data = NULL;
+
+if (!ioapic)
+return;
+
+list = qlist_new();
+
+for (i = 0; i< IOAPIC_NUM_PINS; i++) {
+QObject *obj;
This assumes there is only one ioapic. While I don't think there's a
good reason to add more (the one we have is causing sufficient trouble),
I suggest returning an array of ioapics (or include gsibase: [{ ioapic:
object, gsibase: 0 }, { ioapic: object, gsibase: 24 }].
--
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCHv2] add "info ioapic" monitor command
On Wed, Dec 30, 2009 at 05:04:08PM +0200, Avi Kivity wrote:
> On 12/30/2009 01:50 PM, Gleb Natapov wrote:
> >Knowing ioapic configuration is very useful for the poor soles
> >how need to debug guest occasionally.
> >+
> >+void do_info_ioapic(Monitor *mon, QObject **ret_data)
> >+{
> >+int i;
> >+QList *list;
> >+
> >+*ret_data = NULL;
> >+
> >+if (!ioapic)
> >+return;
> >+
> >+list = qlist_new();
> >+
> >+for (i = 0; i< IOAPIC_NUM_PINS; i++) {
> >+QObject *obj;
>
>
> This assumes there is only one ioapic. While I don't think there's
> a good reason to add more (the one we have is causing sufficient
> trouble), I suggest returning an array of ioapics (or include
> gsibase: [{ ioapic: object, gsibase: 0 }, { ioapic: object, gsibase:
> 24 }].
>
For the case of multiple ioapics I thought about extending the command
to get ioapic id as a parameter when time comes. gsibase is not know to
ioapic itself, so this info does not belong here.
--
Gleb.
Re: [Qemu-devel] [PATCHv2] add "info ioapic" monitor command
On 12/30/2009 05:53 PM, Gleb Natapov wrote:
This assumes there is only one ioapic. While I don't think there's
a good reason to add more (the one we have is causing sufficient
trouble), I suggest returning an array of ioapics (or include
gsibase: [{ ioapic: object, gsibase: 0 }, { ioapic: object, gsibase:
24 }].
For the case of multiple ioapics I thought about extending the command
to get ioapic id as a parameter when time comes. gsibase is not know to
ioapic itself, so this info does not belong here.
Ok.
--
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Re: qemu-kvm-0.12 bug?
On Tuesday 29 December 2009 13:24:36 Luiz Capitulino wrote: > > > Is the "-balloon virtio" parameter passed on the command-line? > > > > # grep balloon /usr/local/var/log/libvirt/qemu/* | wc -l > > 0 > > > > These logs include some history - so the parameter isn't used by libvirt > > even when ballooning works? > > This parameter is new in qemu-0.12. Ah, thx! I informed the libvirt guys. kr,t
Re: [Qemu-devel] [PATCH] debugcon: support for debugging consoles (e.g. Bochs port 0xe9)
On Tue, Dec 29, 2009 at 01:51:36PM -0800, H. Peter Anvin wrote: > Add generic support for debugging consoles (simple I/O ports which > when written to cause debugging output to be written to a target.) > The current implementation matches Bochs' port 0xe9, allowing the same > debugging code to be used for both Bochs and Qemu. > > There is no vm state associated with the debugging port, simply > because it has none -- the entire interface is a single, stateless, > write-only port. > > Most of the code was cribbed from the serial port driver. Hi, SeaBIOS writes debugging info to port 0x0402. Unfortunately, qemu has to be recompiled in order to display this info. Will your patch enable one to get at the 0x0402 data without recompiling? -Kevin
[Qemu-devel] [PATCH] ARM semihosting improvements
From: Daniel Jacobowitz
This patch improves ARM semihosting to the point where qemu-system-arm
can simulate cc1 from GCC. It can't simulate GCC itself, which
requires POSIXy bits like execve, but the backend works, including the
preprocessor.
* Use -kernel and -append for SYS_GET_CMDLINE. This lets semihosted
programs receive command line options.
* Correctly return errno values without gdb attached. Previously
most system calls discarded errno.
* Set errno == ENOENT after SYS_FLEN of a directory. This is a
workaround for the absence of stat in the ARM semihosting protocol.
Now stat on directories will report that they do not exist, which
causes most applications to skip the missing directory.
Signed-off-by: Daniel Jacobowitz
diff --git a/arm-semi.c b/arm-semi.c
index 5239ffc..e4d1ae5 100644
--- a/arm-semi.c
+++ b/arm-semi.c
@@ -35,6 +35,7 @@
#include "qemu-common.h"
#include "sysemu.h"
#include "gdbstub.h"
+#include "hw/arm-misc.h"
#endif
#define SYS_OPEN0x01
@@ -108,8 +109,12 @@ static inline uint32_t set_swi_errno(TaskState *ts,
uint32_t code)
return code;
}
#else
+static target_ulong syscall_err;
+
static inline uint32_t set_swi_errno(CPUState *env, uint32_t code)
{
+if (code == (uint32_t)-1)
+syscall_err = errno;
return code;
}
@@ -118,10 +123,6 @@ static inline uint32_t set_swi_errno(CPUState *env,
uint32_t code)
static target_ulong arm_semi_syscall_len;
-#if !defined(CONFIG_USER_ONLY)
-static target_ulong syscall_err;
-#endif
-
static void arm_semi_cb(CPUState *env, target_ulong ret, target_ulong err)
{
#ifdef CONFIG_USER_ONLY
@@ -156,8 +157,17 @@ static void arm_semi_flen_cb(CPUState *env, target_ulong
ret, target_ulong err)
{
/* The size is always stored in big-endian order, extract
the value. We assume the size always fit in 32 bits. */
-uint32_t size;
+uint32_t size, mode;
cpu_memory_rw_debug(env, env->regs[13]-64+32, (uint8_t *)&size, 4, 0);
+
+/* Report that all directories do not exist. */
+cpu_memory_rw_debug(env, env->regs[13]-64+8, (uint8_t *)&mode, 4, 0);
+mode = be32_to_cpu(mode);
+if (mode & 04) {
+err = 2;
+size = -1;
+}
+
env->regs[0] = be32_to_cpu(size);
#ifdef CONFIG_USER_ONLY
((TaskState *)env->opaque)->swi_errno = err;
@@ -310,6 +320,11 @@ uint32_t do_arm_semihosting(CPUState *env)
ret = set_swi_errno(ts, fstat(ARG(0), &buf));
if (ret == (uint32_t)-1)
return -1;
+if (S_ISDIR (buf.st_mode)) {
+errno = ENOENT;
+set_swi_errno(ts, -1);
+return -1;
+}
return buf.st_size;
}
case SYS_TMPNAM:
@@ -370,13 +385,21 @@ uint32_t do_arm_semihosting(CPUState *env)
return syscall_err;
#endif
case SYS_GET_CMDLINE:
-#ifdef CONFIG_USER_ONLY
-/* Build a commandline from the original argv. */
{
-char **arg = ts->info->host_argv;
int len = ARG(1);
/* lock the buffer on the ARM side */
char *cmdline_buffer = (char*)lock_user(VERIFY_WRITE, ARG(0), len,
0);
+#ifdef CONFIG_USER_ONLY
+/* Build a commandline from the original argv. */
+char **arg = ts->info->host_argv;
+#else
+/* Build a commandline from -kernel and -append. */
+/* This is simple but only because we do no escaping. */
+const char *arglist[3] = { 0 }, **arg = arglist;
+
+arglist[0] = env->boot_info->kernel_filename;
+arglist[1] = env->boot_info->kernel_cmdline;
+#endif
if (!cmdline_buffer)
/* FIXME - should this error code be -TARGET_EFAULT ? */
@@ -410,9 +433,6 @@ uint32_t do_arm_semihosting(CPUState *env)
/* Return success if commandline fit into buffer. */
return *arg ? -1 : 0;
}
-#else
- return -1;
-#endif
case SYS_HEAPINFO:
{
uint32_t *ptr;
--
Daniel Jacobowitz
CodeSourcery
Re: [Qemu-devel] [PATCH] ARM semihosting improvements
On Wed, Dec 30, 2009 at 6:23 PM, Daniel Jacobowitz wrote:
> From: Daniel Jacobowitz
>
> This patch improves ARM semihosting to the point where qemu-system-arm
> can simulate cc1 from GCC. It can't simulate GCC itself, which
> requires POSIXy bits like execve, but the backend works, including the
> preprocessor.
>
> * Use -kernel and -append for SYS_GET_CMDLINE. This lets semihosted
> programs receive command line options.
>
> * Correctly return errno values without gdb attached. Previously
> most system calls discarded errno.
>
> * Set errno == ENOENT after SYS_FLEN of a directory. This is a
> workaround for the absence of stat in the ARM semihosting protocol.
> Now stat on directories will report that they do not exist, which
> causes most applications to skip the missing directory.
>
> Signed-off-by: Daniel Jacobowitz
>
> diff --git a/arm-semi.c b/arm-semi.c
> index 5239ffc..e4d1ae5 100644
> --- a/arm-semi.c
> +++ b/arm-semi.c
> @@ -35,6 +35,7 @@
> #include "qemu-common.h"
> #include "sysemu.h"
> #include "gdbstub.h"
> +#include "hw/arm-misc.h"
> #endif
>
> #define SYS_OPEN 0x01
> @@ -108,8 +109,12 @@ static inline uint32_t set_swi_errno(TaskState *ts,
> uint32_t code)
> return code;
> }
> #else
> +static target_ulong syscall_err;
> +
> static inline uint32_t set_swi_errno(CPUState *env, uint32_t code)
> {
> + if (code == (uint32_t)-1)
> + syscall_err = errno;
> return code;
> }
>
> @@ -118,10 +123,6 @@ static inline uint32_t set_swi_errno(CPUState *env,
> uint32_t code)
>
> static target_ulong arm_semi_syscall_len;
>
> -#if !defined(CONFIG_USER_ONLY)
> -static target_ulong syscall_err;
> -#endif
> -
> static void arm_semi_cb(CPUState *env, target_ulong ret, target_ulong err)
> {
> #ifdef CONFIG_USER_ONLY
> @@ -156,8 +157,17 @@ static void arm_semi_flen_cb(CPUState *env, target_ulong
> ret, target_ulong err)
> {
> /* The size is always stored in big-endian order, extract
> the value. We assume the size always fit in 32 bits. */
> - uint32_t size;
> + uint32_t size, mode;
> cpu_memory_rw_debug(env, env->regs[13]-64+32, (uint8_t *)&size, 4, 0);
> +
> + /* Report that all directories do not exist. */
> + cpu_memory_rw_debug(env, env->regs[13]-64+8, (uint8_t *)&mode, 4, 0);
> + mode = be32_to_cpu(mode);
> + if (mode & 04) {
> + err = 2;
> + size = -1;
> + }
> +
> env->regs[0] = be32_to_cpu(size);
> #ifdef CONFIG_USER_ONLY
> ((TaskState *)env->opaque)->swi_errno = err;
> @@ -310,6 +320,11 @@ uint32_t do_arm_semihosting(CPUState *env)
> ret = set_swi_errno(ts, fstat(ARG(0), &buf));
> if (ret == (uint32_t)-1)
> return -1;
> + if (S_ISDIR (buf.st_mode)) {
> + errno = ENOENT;
> + set_swi_errno(ts, -1);
> + return -1;
> + }
> return buf.st_size;
> }
> case SYS_TMPNAM:
> @@ -370,13 +385,21 @@ uint32_t do_arm_semihosting(CPUState *env)
> return syscall_err;
> #endif
> case SYS_GET_CMDLINE:
> -#ifdef CONFIG_USER_ONLY
> - /* Build a commandline from the original argv. */
> {
> - char **arg = ts->info->host_argv;
> int len = ARG(1);
> /* lock the buffer on the ARM side */
> char *cmdline_buffer = (char*)lock_user(VERIFY_WRITE, ARG(0),
> len, 0);
> +#ifdef CONFIG_USER_ONLY
> + /* Build a commandline from the original argv. */
> + char **arg = ts->info->host_argv;
Did you check this works? I think it doesn't since host_argv
field is being assigned target_argv, defined in main. And
target_argv is freed in main before starting simulation...
Laurent
> +#else
> + /* Build a commandline from -kernel and -append. */
> + /* This is simple but only because we do no escaping. */
> + const char *arglist[3] = { 0 }, **arg = arglist;
> +
> + arglist[0] = env->boot_info->kernel_filename;
> + arglist[1] = env->boot_info->kernel_cmdline;
> +#endif
>
> if (!cmdline_buffer)
> /* FIXME - should this error code be -TARGET_EFAULT ? */
> @@ -410,9 +433,6 @@ uint32_t do_arm_semihosting(CPUState *env)
> /* Return success if commandline fit into buffer. */
> return *arg ? -1 : 0;
> }
> -#else
> - return -1;
> -#endif
> case SYS_HEAPINFO:
> {
> uint32_t *ptr;
>
> --
> Daniel Jacobowitz
> CodeSourcery
>
>
>
Re: [Qemu-devel] [PATCH] ARM semihosting improvements
On Wed, Dec 30, 2009 at 06:32:14PM +0100, Laurent Desnogues wrote:
> > @@ -370,13 +385,21 @@ uint32_t do_arm_semihosting(CPUState *env)
> > return syscall_err;
> > #endif
> > case SYS_GET_CMDLINE:
> > -#ifdef CONFIG_USER_ONLY
> > - /* Build a commandline from the original argv. */
> > {
> > - char **arg = ts->info->host_argv;
> > int len = ARG(1);
> > /* lock the buffer on the ARM side */
> > char *cmdline_buffer = (char*)lock_user(VERIFY_WRITE, ARG(0),
> > len, 0);
> > +#ifdef CONFIG_USER_ONLY
> > + /* Build a commandline from the original argv. */
> > + char **arg = ts->info->host_argv;
>
> Did you check this works? I think it doesn't since host_argv
> field is being assigned target_argv, defined in main. And
> target_argv is freed in main before starting simulation...
Well, that's possible - but that code was there already; I only moved
the CONFIG_USER_ONLY case down a couple of lines.
I don't recall why there's user-mode support in this file.
--
Daniel Jacobowitz
CodeSourcery
Re: [Qemu-devel] [Bug] qemu-system-ppc: "invalid/unsupported opcode" during debug session
On Wed, Dec 30, 2009 at 03:39:32PM +0100, Alexander Graf wrote: > > Am 29.12.2009 um 22:07 schrieb Aurelien Jarno : > >> On Tue, Dec 29, 2009 at 04:09:17PM +0100, Stefan Weil wrote: >>> Test environment: >>> >>> * ppc-softmmu/qemu-system-ppc running on x86_64 host >>> * emulated ppc is running debian lenny >>> >>> >>> >>> While debugging on the emulated ppc (each time when >>> a shared library is loaded after "r" command?), >>> qemu-system-ppc prints this error message: >>> >>> invalid/unsupported opcode: 00 - 00 - 00 () 4800fa44 1 >>> >>> >>> >>> If logging is enabled, the error message goes to qemu.log: >>> >>> IN: >>> 0xc0013488: nop >>> 0xc001348c: rlwinm r3,r3,0,0,19 >>> 0xc0013490: li r4,128 >>> 0xc0013494: mtctr r4 >>> 0xc0013498: mr r6,r3 >>> 0xc001349c: dcbst r0,r3 >>> >>> invalid/unsupported opcode: 00 - 00 - 00 () 4800fa44 1 >>> IN: >>> 0x4800fa40: twger2,r2 >>> 0x4800fa44: .long 0x0 >>> >> >> The problem is that QEMU doesn't stop the decoding of instructions >> when >> it encounters a trap instruction. We should probably either end the TB >> in that case, or avoid printing "invalid/unsupported opcode", as this >> instruction will actually never been executed. > > Given how seldom they occur, it's probably best (easiest to read) to end > the TB. > The question is to know if there are other conditions than branches and trap where code can be translated, but then never executed. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH] ARM semihosting improvements
From: Daniel Jacobowitz
This patch improves ARM semihosting to the point where qemu-system-arm
can simulate cc1 from GCC. It can't simulate GCC itself, which
requires POSIXy bits like execve, but the backend works, including the
preprocessor.
* Use -kernel and -append for SYS_GET_CMDLINE. This lets semihosted
programs receive command line options.
* Correctly return errno values without gdb attached. Previously
most system calls discarded errno.
* Set errno == ENOENT after SYS_FLEN of a directory. This is a
workaround for the absence of stat in the ARM semihosting protocol.
Now stat on directories will report that they do not exist, which
causes most applications to skip the missing directory.
Also fixes a use-after-free when using semihosting with linux-user,
as pointed out by Laurent Desnogues.
Signed-off-by: Daniel Jacobowitz
diff --git a/arm-semi.c b/arm-semi.c
index 5239ffc..e4d1ae5 100644
--- a/arm-semi.c
+++ b/arm-semi.c
@@ -35,6 +35,7 @@
#include "qemu-common.h"
#include "sysemu.h"
#include "gdbstub.h"
+#include "hw/arm-misc.h"
#endif
#define SYS_OPEN0x01
@@ -108,8 +109,12 @@ static inline uint32_t set_swi_errno(TaskState *ts,
uint32_t code)
return code;
}
#else
+static target_ulong syscall_err;
+
static inline uint32_t set_swi_errno(CPUState *env, uint32_t code)
{
+if (code == (uint32_t)-1)
+syscall_err = errno;
return code;
}
@@ -118,10 +123,6 @@ static inline uint32_t set_swi_errno(CPUState *env,
uint32_t code)
static target_ulong arm_semi_syscall_len;
-#if !defined(CONFIG_USER_ONLY)
-static target_ulong syscall_err;
-#endif
-
static void arm_semi_cb(CPUState *env, target_ulong ret, target_ulong err)
{
#ifdef CONFIG_USER_ONLY
@@ -156,8 +157,17 @@ static void arm_semi_flen_cb(CPUState *env, target_ulong
ret, target_ulong err)
{
/* The size is always stored in big-endian order, extract
the value. We assume the size always fit in 32 bits. */
-uint32_t size;
+uint32_t size, mode;
cpu_memory_rw_debug(env, env->regs[13]-64+32, (uint8_t *)&size, 4, 0);
+
+/* Report that all directories do not exist. */
+cpu_memory_rw_debug(env, env->regs[13]-64+8, (uint8_t *)&mode, 4, 0);
+mode = be32_to_cpu(mode);
+if (mode & 04) {
+err = 2;
+size = -1;
+}
+
env->regs[0] = be32_to_cpu(size);
#ifdef CONFIG_USER_ONLY
((TaskState *)env->opaque)->swi_errno = err;
@@ -310,6 +320,11 @@ uint32_t do_arm_semihosting(CPUState *env)
ret = set_swi_errno(ts, fstat(ARG(0), &buf));
if (ret == (uint32_t)-1)
return -1;
+if (S_ISDIR (buf.st_mode)) {
+errno = ENOENT;
+set_swi_errno(ts, -1);
+return -1;
+}
return buf.st_size;
}
case SYS_TMPNAM:
@@ -370,13 +385,21 @@ uint32_t do_arm_semihosting(CPUState *env)
return syscall_err;
#endif
case SYS_GET_CMDLINE:
-#ifdef CONFIG_USER_ONLY
-/* Build a commandline from the original argv. */
{
-char **arg = ts->info->host_argv;
int len = ARG(1);
/* lock the buffer on the ARM side */
char *cmdline_buffer = (char*)lock_user(VERIFY_WRITE, ARG(0), len,
0);
+#ifdef CONFIG_USER_ONLY
+/* Build a commandline from the original argv. */
+char **arg = ts->info->host_argv;
+#else
+/* Build a commandline from -kernel and -append. */
+/* This is simple but only because we do no escaping. */
+const char *arglist[3] = { 0 }, **arg = arglist;
+
+arglist[0] = env->boot_info->kernel_filename;
+arglist[1] = env->boot_info->kernel_cmdline;
+#endif
if (!cmdline_buffer)
/* FIXME - should this error code be -TARGET_EFAULT ? */
@@ -410,9 +433,6 @@ uint32_t do_arm_semihosting(CPUState *env)
/* Return success if commandline fit into buffer. */
return *arg ? -1 : 0;
}
-#else
- return -1;
-#endif
case SYS_HEAPINFO:
{
uint32_t *ptr;
diff --git a/linux-user/main.c b/linux-user/main.c
index a0d8ce7..fbff44a 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -2782,11 +2782,6 @@ int main(int argc, char **argv, char **envp)
_exit(1);
}
-for (i = 0; i < target_argc; i++) {
-free(target_argv[i]);
-}
-free(target_argv);
-
for (wrk = target_environ; *wrk; wrk++) {
free(*wrk);
}
--
Daniel Jacobowitz
CodeSourcery
Re: [Qemu-devel] [Bug] qemu-system-ppc: "invalid/unsupported opcode" during debug session
Am 30.12.2009 um 18:46 schrieb Aurelien Jarno : On Wed, Dec 30, 2009 at 03:39:32PM +0100, Alexander Graf wrote: Am 29.12.2009 um 22:07 schrieb Aurelien Jarno : On Tue, Dec 29, 2009 at 04:09:17PM +0100, Stefan Weil wrote: Test environment: * ppc-softmmu/qemu-system-ppc running on x86_64 host * emulated ppc is running debian lenny While debugging on the emulated ppc (each time when a shared library is loaded after "r" command?), qemu-system-ppc prints this error message: invalid/unsupported opcode: 00 - 00 - 00 () 4800fa44 1 If logging is enabled, the error message goes to qemu.log: IN: 0xc0013488: nop 0xc001348c: rlwinm r3,r3,0,0,19 0xc0013490: li r4,128 0xc0013494: mtctr r4 0xc0013498: mr r6,r3 0xc001349c: dcbst r0,r3 invalid/unsupported opcode: 00 - 00 - 00 () 4800fa44 1 IN: 0x4800fa40: twger2,r2 0x4800fa44: .long 0x0 The problem is that QEMU doesn't stop the decoding of instructions when it encounters a trap instruction. We should probably either end the TB in that case, or avoid printing "invalid/unsupported opcode", as this instruction will actually never been executed. Given how seldom they occur, it's probably best (easiest to read) to end the TB. The question is to know if there are other conditions than branches and trap where code can be translated, but then never executed. We don't fix that by hacking the invalid opcode print either, because we'd still have to mark instructions we can't determine if an instruction is invalid later on. IMHO the best solution would actually be to just not print out anything except for qemu.log if -d is used. Alex
Re: [Qemu-devel] [PATCH] ARM semihosting improvements
Daniel Jacobowitz wrote: > From: Daniel Jacobowitz > > This patch improves ARM semihosting to the point where qemu-system-arm > can simulate cc1 from GCC. It can't simulate GCC itself, which > requires POSIXy bits like execve, but the backend works, including the > preprocessor. I see that you didn't start the semihosting support, but what's the purpose of it? Why would you use it to run programs like cc1 instead of qemu-arm, the user mode simulation? Thanks, -- Jamie
Re: [Qemu-devel] [PATCH] ARM semihosting improvements
On Wed, Dec 30, 2009 at 06:38:16PM +, Jamie Lokier wrote: > I see that you didn't start the semihosting support, but what's the > purpose of it? Why would you use it to run programs like cc1 instead > of qemu-arm, the user mode simulation? You use it to run an arm-none-eabi cc1, not an arm-none-linux-gnueabi cc1. In my case that was useful because there was no dynamic memory allocation. We also rely on semihosting for GCC testing. -- Daniel Jacobowitz CodeSourcery
Re: [Qemu-devel] [PATCH] add "info ioapic" monitor command
On 12/30/2009 08:19 AM, Avi Kivity wrote: On 12/30/2009 04:17 PM, Anthony Liguori wrote: That's a good idea, but it would mean that we'd need into ioapic and info ioapic-kvm if we go for a split implementation. I don't think that's a problem. A developer will be smart enough to know that they need to do that (and will get an error if they don't). I wasn't worried about that, only the increased code duplication. I wasn't thinking there would be an info ioapic command but rather a generic device-info command that would work with any qdev device. No code duplication (in fact, much, much less in the long term). Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH] add "info ioapic" monitor command
On 12/30/2009 08:30 AM, Gleb Natapov wrote: On Wed, Dec 30, 2009 at 04:26:11PM +0200, Gleb Natapov wrote: On Wed, Dec 30, 2009 at 08:16:30AM -0600, Anthony Liguori wrote: On 12/30/2009 03:27 AM, Gleb Natapov wrote: On Tue, Dec 29, 2009 at 08:00:09PM -0600, Anthony Liguori wrote: On 12/24/2009 09:02 AM, Gleb Natapov wrote: Knowing ioapic configuration is very useful for the poor soles how need to debug guest occasionally. Can this be implemented in terms of VMState? What do you mean and why it whould be any better? We've been discussing having a command that's something like 'info-device ioapic' that would dump the device state based on what's stored in VMState for the device. The advantage of a command like this is that it works for any qdev device making it much more useful. The disadvantage is that at the moment, we store ioredtbl as an array of uint64s. To get the level of info you're looking for we would need to teach VMState how to decode this structure. So this is completely orthogonal to my patch. There are devices already that have info command. No need to hold the patch just because sometime in the feature VMState may be extended. Other then that adding print function to VMState sounds like a good idea. BTW I'd like this to be backported to 0.12 too. It's not a bug fix so that doesn't make sense. Regards, Anthony Liguori -- Gleb.
Re: [Qemu-devel] [PATCH] add "info ioapic" monitor command
On 12/30/2009 08:26 AM, Gleb Natapov wrote: On Wed, Dec 30, 2009 at 08:16:30AM -0600, Anthony Liguori wrote: On 12/30/2009 03:27 AM, Gleb Natapov wrote: On Tue, Dec 29, 2009 at 08:00:09PM -0600, Anthony Liguori wrote: On 12/24/2009 09:02 AM, Gleb Natapov wrote: Knowing ioapic configuration is very useful for the poor soles how need to debug guest occasionally. Can this be implemented in terms of VMState? What do you mean and why it whould be any better? We've been discussing having a command that's something like 'info-device ioapic' that would dump the device state based on what's stored in VMState for the device. The advantage of a command like this is that it works for any qdev device making it much more useful. The disadvantage is that at the moment, we store ioredtbl as an array of uint64s. To get the level of info you're looking for we would need to teach VMState how to decode this structure. So this is completely orthogonal to my patch. There are devices already that have info command. The other commands were added before we had the ability to do it right. Now we can do it right and there's really no reason not to. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCHv2] add "info ioapic" monitor command
On 12/30/2009 09:58 AM, Avi Kivity wrote:
On 12/30/2009 05:53 PM, Gleb Natapov wrote:
This assumes there is only one ioapic. While I don't think there's
a good reason to add more (the one we have is causing sufficient
trouble), I suggest returning an array of ioapics (or include
gsibase: [{ ioapic: object, gsibase: 0 }, { ioapic: object, gsibase:
24 }].
For the case of multiple ioapics I thought about extending the command
to get ioapic id as a parameter when time comes. gsibase is not know to
ioapic itself, so this info does not belong here.
Ok.
Here's a good example of why attacking device-info is a better general
approach.
Right now, you only support one ioapic and you only display a portion of
the device's state. In the future, it's likely that you'll want to
support multiple ioapic (okay, maybe not likely, but possible), and it's
very likely you'll want to include more state for the ioapic.
By definition, VMState contains all of the state of a device that's
guest visible. That means from a debugging perspective, you are
guaranteed to have all of the information you need for the particular
device. Also, because it's necessarily to save multiple instances of a
device with VMState, it automatically supports the ability to view
multiple instances of a device.
Regards,
Anthony Liguori
Re: [Qemu-devel] [PATCHv2] add "info ioapic" monitor command
On 12/30/2009 06:13 AM, Gleb Natapov wrote: On Wed, Dec 30, 2009 at 02:08:27PM +0200, Izik Eidus wrote: On Wed, 30 Dec 2009 14:05:10 +0200 Gleb Natapov wrote: On Wed, Dec 30, 2009 at 12:01:28PM +, Blue Swirl wrote: On Wed, Dec 30, 2009 at 11:50 AM, Gleb Natapov wrote: Knowing ioapic configuration is very useful for the poor soles how need to debug guest occasionally. +static struct IOAPICState *ioapic; Ugly. Agree And now it seems like even you dont agree with me!!! I agree, but all other info functions in qemu do exactly same: referencing global data. This is ugly _and_ this nothing to do with my patch. This goes away if you use VMState since we already have a global table that contains all of the registered VMState instances. It also maps device names/instance ids to the actual field contents. Regards, Anthony Liguori -- Gleb.
[Qemu-devel] qemu/qemu-kvm-0.12.1.2 - migration still very slow
Hi, few months ago Pierre Riteau reported regression of exec migration in qemu. (http://lists.gnu.org/archive/html/qemu-devel/2009-08/msg01557.html) There was some discussion, but there is no clear conclusion. Today, I tried qemu-kvm-0.12.1.2 and migration is still very slow (using libvirt save command, which internaly uses migration) to suspend vm with 4GB of ram can take almost half an hour, although system is not loaded at all. Is there some way to speed things up? Any tips would be greatly appreciated... with best regards nik -- - Nikola CIPRICH LinuxBox.cz, s.r.o. 28. rijna 168, 709 01 Ostrava tel.: +420 596 603 142 fax:+420 596 621 273 mobil: +420 777 093 799 www.linuxbox.cz mobil servis: +420 737 238 656 email servis: ser...@linuxbox.cz -
Re: [Qemu-devel] [PATCH] add "info ioapic" monitor command
On Wed, Dec 30, 2009 at 04:49:01PM -0600, Anthony Liguori wrote: > On 12/30/2009 08:26 AM, Gleb Natapov wrote: > >On Wed, Dec 30, 2009 at 08:16:30AM -0600, Anthony Liguori wrote: > >>On 12/30/2009 03:27 AM, Gleb Natapov wrote: > >>>On Tue, Dec 29, 2009 at 08:00:09PM -0600, Anthony Liguori wrote: > On 12/24/2009 09:02 AM, Gleb Natapov wrote: > >Knowing ioapic configuration is very useful for the poor soles > >how need to debug guest occasionally. > > Can this be implemented in terms of VMState? > > >>>What do you mean and why it whould be any better? > >> > >>We've been discussing having a command that's something like > >>'info-device ioapic' that would dump the device state based on > >>what's stored in VMState for the device. > >> > >>The advantage of a command like this is that it works for any qdev > >>device making it much more useful. > >> > >>The disadvantage is that at the moment, we store ioredtbl as an > >>array of uint64s. To get the level of info you're looking for we > >>would need to teach VMState how to decode this structure. > >> > > > >So this is completely orthogonal to my patch. There are devices already > >that have info command. > > The other commands were added before we had the ability to do it > right. Now we can do it right and there's really no reason not to. > No we don't. You want to add the infrastructure to do it right and I have no problem with that go add it. But till then we do it old way. -- Gleb.
Re: [Qemu-devel] [PATCH] add "info ioapic" monitor command
On Wed, Dec 30, 2009 at 04:47:45PM -0600, Anthony Liguori wrote: > On 12/30/2009 08:30 AM, Gleb Natapov wrote: > >On Wed, Dec 30, 2009 at 04:26:11PM +0200, Gleb Natapov wrote: > >>On Wed, Dec 30, 2009 at 08:16:30AM -0600, Anthony Liguori wrote: > >>>On 12/30/2009 03:27 AM, Gleb Natapov wrote: > On Tue, Dec 29, 2009 at 08:00:09PM -0600, Anthony Liguori wrote: > >On 12/24/2009 09:02 AM, Gleb Natapov wrote: > >>Knowing ioapic configuration is very useful for the poor soles > >>how need to debug guest occasionally. > > > >Can this be implemented in terms of VMState? > > > What do you mean and why it whould be any better? > >>> > >>>We've been discussing having a command that's something like > >>>'info-device ioapic' that would dump the device state based on > >>>what's stored in VMState for the device. > >>> > >>>The advantage of a command like this is that it works for any qdev > >>>device making it much more useful. > >>> > >>>The disadvantage is that at the moment, we store ioredtbl as an > >>>array of uint64s. To get the level of info you're looking for we > >>>would need to teach VMState how to decode this structure. > >>> > >> > >>So this is completely orthogonal to my patch. There are devices already > >>that have info command. No need to hold the patch just because sometime > >>in the feature VMState may be extended. Other then that adding print > >>function to VMState sounds like a good idea. > >BTW I'd like this to be backported to 0.12 too. > > It's not a bug fix so that doesn't make sense. > It helps debug problems and it is not intrusive. Since 0.12 will be used for a long time I want to add it there for convenience. -- Gleb.
Re: [Qemu-devel] [PATCHv2] add "info ioapic" monitor command
On Wed, Dec 30, 2009 at 05:04:53PM -0600, Anthony Liguori wrote:
> On 12/30/2009 09:58 AM, Avi Kivity wrote:
> >On 12/30/2009 05:53 PM, Gleb Natapov wrote:
> >>
> >>>This assumes there is only one ioapic. While I don't think there's
> >>>a good reason to add more (the one we have is causing sufficient
> >>>trouble), I suggest returning an array of ioapics (or include
> >>>gsibase: [{ ioapic: object, gsibase: 0 }, { ioapic: object, gsibase:
> >>>24 }].
> >>>
> >>For the case of multiple ioapics I thought about extending the command
> >>to get ioapic id as a parameter when time comes. gsibase is not know to
> >>ioapic itself, so this info does not belong here.
> >>
> >
> >Ok.
>
> Here's a good example of why attacking device-info is a better
> general approach.
>
> Right now, you only support one ioapic and you only display a
> portion of the device's state. In the future, it's likely that
> you'll want to support multiple ioapic (okay, maybe not likely, but
> possible), and it's very likely you'll want to include more state
> for the ioapic.
I included only the state I need for debugging. I don't what to see
complete state. It will just clatter important info.
>
> By definition, VMState contains all of the state of a device that's
> guest visible. That means from a debugging perspective, you are
> guaranteed to have all of the information you need for the
> particular device. Also, because it's necessarily to save multiple
> instances of a device with VMState, it automatically supports the
> ability to view multiple instances of a device.
>
--
Gleb.
Re: [Qemu-devel] [PATCHv2] add "info ioapic" monitor command
On Wed, Dec 30, 2009 at 05:07:14PM -0600, Anthony Liguori wrote: > On 12/30/2009 06:13 AM, Gleb Natapov wrote: > >On Wed, Dec 30, 2009 at 02:08:27PM +0200, Izik Eidus wrote: > >>On Wed, 30 Dec 2009 14:05:10 +0200 > >>Gleb Natapov wrote: > >> > >>>On Wed, Dec 30, 2009 at 12:01:28PM +, Blue Swirl wrote: > On Wed, Dec 30, 2009 at 11:50 AM, Gleb Natapov wrote: > >Knowing ioapic configuration is very useful for the poor soles > >how need to debug guest occasionally. > > >+static struct IOAPICState *ioapic; > > Ugly. > > >>>Agree > >> > >>And now it seems like even you dont agree with me!!! > >> > >I agree, but all other info functions in qemu do exactly same: > >referencing global data. This is ugly _and_ this nothing to > >do with my patch. > > This goes away if you use VMState since we already have a global > table that contains all of the registered VMState instances. It > also maps device names/instance ids to the actual field contents. > I've got you point. When VMState will have this cool powers you are talking about I'll use it. I don't see the point of bragging about something that doesn't exists. -- Gleb.
[Qemu-devel] [PATCH 01/14] Introduce qemu_write_full()
A variant of write(2) which handles partial write.
Signed-off-by: Kirill A. Shutemov
---
osdep.c | 27 +++
qemu-common.h |1 +
2 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/osdep.c b/osdep.c
index e4836e7..d2406f2 100644
--- a/osdep.c
+++ b/osdep.c
@@ -243,6 +243,33 @@ int qemu_open(const char *name, int flags, ...)
return ret;
}
+/*
+ * A variant of write(2) which handles partial write.
+ *
+ * Return the number of bytes transferred.
+ * Set errno if fewer than `count' bytes are written.
+ */
+ssize_t qemu_write_full(int fd, const void *buf, size_t count)
+{
+ssize_t ret = 0;
+ssize_t total = 0;
+
+while (count) {
+ret = write(fd, buf, count);
+if (ret < 0) {
+if (errno == EINTR)
+continue;
+break;
+}
+
+count -= ret;
+buf += ret;
+ total += ret;
+}
+
+return total;
+}
+
#ifndef _WIN32
/*
* Creates a pipe with FD_CLOEXEC set on both file descriptors
diff --git a/qemu-common.h b/qemu-common.h
index 8630f8c..a8144cb 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -160,6 +160,7 @@ void qemu_mutex_lock_iothread(void);
void qemu_mutex_unlock_iothread(void);
int qemu_open(const char *name, int flags, ...);
+ssize_t qemu_write_full(int fd, const void *buf, size_t count);
void qemu_set_cloexec(int fd);
#ifndef _WIN32
--
1.6.5.7
[Qemu-devel] [PATCH 02/14] posix-aio-compat.c: fix warning with _FORTIFY_SOURCE
CCposix-aio-compat.o
cc1: warnings being treated as errors
posix-aio-compat.c: In function 'aio_signal_handler':
posix-aio-compat.c:505: error: ignoring return value of 'write', declared with
attribute warn_unused_result
make: *** [posix-aio-compat.o] Error 1
Signed-off-by: Kirill A. Shutemov
---
posix-aio-compat.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/posix-aio-compat.c b/posix-aio-compat.c
index dc14f53..e0d71fa 100644
--- a/posix-aio-compat.c
+++ b/posix-aio-compat.c
@@ -501,8 +501,11 @@ static void aio_signal_handler(int signum)
{
if (posix_aio_state) {
char byte = 0;
+int ret;
-write(posix_aio_state->wfd, &byte, sizeof(byte));
+ret = write(posix_aio_state->wfd, &byte, sizeof(byte));
+if (ret < 0 && errno != EAGAIN)
+die("write()");
}
qemu_service_io();
--
1.6.5.7
[Qemu-devel] [PATCH 03/14] block/cow.c: fix warnings with _FORTIFY_SOURCE
CCblock/cow.o
cc1: warnings being treated as errors
block/cow.c: In function 'cow_create':
block/cow.c:251: error: ignoring return value of 'write', declared with
attribute warn_unused_result
block/cow.c:253: error: ignoring return value of 'ftruncate', declared with
attribute warn_unused_result
make: *** [block/cow.o] Error 1
Signed-off-by: Kirill A. Shutemov
---
block/cow.c | 19 ---
1 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/block/cow.c b/block/cow.c
index a70854e..ba07b97 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -209,6 +209,7 @@ static int cow_create(const char *filename,
QEMUOptionParameter *options)
struct stat st;
int64_t image_sectors = 0;
const char *image_filename = NULL;
+int ret;
/* Read out options */
while (options && options->name) {
@@ -248,11 +249,23 @@ static int cow_create(const char *filename,
QEMUOptionParameter *options)
}
cow_header.sectorsize = cpu_to_be32(512);
cow_header.size = cpu_to_be64(image_sectors * 512);
-write(cow_fd, &cow_header, sizeof(cow_header));
+ret = qemu_write_full(cow_fd, &cow_header, sizeof(cow_header));
+if (ret != sizeof(cow_header)) {
+ret = -errno;
+goto exit;
+}
+
/* resize to include at least all the bitmap */
-ftruncate(cow_fd, sizeof(cow_header) + ((image_sectors + 7) >> 3));
+ret = ftruncate(cow_fd, sizeof(cow_header) + ((image_sectors + 7) >> 3));
+if (ret) {
+ret = -errno;
+goto exit;
+}
+
+ret = 0;
+exit:
close(cow_fd);
-return 0;
+return ret;
}
static void cow_flush(BlockDriverState *bs)
--
1.6.5.7
[Qemu-devel] [PATCH 04/14] block/qcow.c: fix warnings with _FORTIFY_SOURCE
CCblock/qcow.o
cc1: warnings being treated as errors
block/qcow.c: In function 'qcow_create':
block/qcow.c:804: error: ignoring return value of 'write', declared with
attribute warn_unused_result
block/qcow.c:806: error: ignoring return value of 'write', declared with
attribute warn_unused_result
block/qcow.c:811: error: ignoring return value of 'write', declared with
attribute warn_unused_result
make: *** [block/qcow.o] Error 1
Signed-off-by: Kirill A. Shutemov
---
block/qcow.c | 26 ++
1 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/block/qcow.c b/block/qcow.c
index 7fc85ae..472494c 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -750,6 +750,7 @@ static int qcow_create(const char *filename,
QEMUOptionParameter *options)
int64_t total_size = 0;
const char *backing_file = NULL;
int flags = 0;
+int ret;
/* Read out options */
while (options && options->name) {
@@ -801,17 +802,34 @@ static int qcow_create(const char *filename,
QEMUOptionParameter *options)
}
/* write all the data */
-write(fd, &header, sizeof(header));
+ret = qemu_write_full(fd, &header, sizeof(header));
+if (ret != sizeof(header)) {
+ret = -errno;
+goto exit;
+}
+
if (backing_file) {
-write(fd, backing_file, backing_filename_len);
+ret = qemu_write_full(fd, backing_file, backing_filename_len);
+if (ret != backing_filename_len) {
+ret = -errno;
+goto exit;
+}
+
}
lseek(fd, header_size, SEEK_SET);
tmp = 0;
for(i = 0;i < l1_size; i++) {
-write(fd, &tmp, sizeof(tmp));
+ret = qemu_write_full(fd, &tmp, sizeof(tmp));
+if (ret != sizeof(tmp)) {
+ret = -errno;
+goto exit;
+}
}
+
+ret = 0;
+exit:
close(fd);
-return 0;
+return ret;
}
static int qcow_make_empty(BlockDriverState *bs)
--
1.6.5.7
[Qemu-devel] [PATCH 05/14] block/vmdk.o: fix warnings with _FORTIFY_SOURCE
CCblock/vmdk.o
cc1: warnings being treated as errors
block/vmdk.c: In function 'vmdk_snapshot_create':
block/vmdk.c:236: error: ignoring return value of 'ftruncate', declared with
attribute warn_unused_result
block/vmdk.c: In function 'vmdk_create':
block/vmdk.c:775: error: ignoring return value of 'write', declared with
attribute warn_unused_result
block/vmdk.c:776: error: ignoring return value of 'write', declared with
attribute warn_unused_result
block/vmdk.c:778: error: ignoring return value of 'ftruncate', declared with
attribute warn_unused_result
block/vmdk.c:784: error: ignoring return value of 'write', declared with
attribute warn_unused_result
block/vmdk.c:790: error: ignoring return value of 'write', declared with
attribute warn_unused_result
block/vmdk.c:807: error: ignoring return value of 'write', declared with
attribute warn_unused_result
make: *** [block/vmdk.o] Error 1
Signed-off-by: Kirill A. Shutemov
---
block/vmdk.c | 50 --
1 files changed, 40 insertions(+), 10 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 4e48622..58fc04b 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -233,7 +233,8 @@ static int vmdk_snapshot_create(const char *filename, const
char *backing_file)
memset(&header, 0, sizeof(header));
memcpy(&header,&hdr[4], sizeof(header)); // skip the VMDK4_MAGIC
-ftruncate(snp_fd, header.grain_offset << 9);
+if (ftruncate(snp_fd, header.grain_offset << 9))
+goto fail;
/* the descriptor offset = 0x200 */
if (lseek(p_fd, 0x200, SEEK_SET) == -1)
goto fail;
@@ -716,6 +717,7 @@ static int vmdk_create(const char *filename,
QEMUOptionParameter *options)
int64_t total_size = 0;
const char *backing_file = NULL;
int flags = 0;
+int ret;
// Read out options
while (options && options->name) {
@@ -772,22 +774,44 @@ static int vmdk_create(const char *filename,
QEMUOptionParameter *options)
header.check_bytes[3] = 0xa;
/* write all the data */
-write(fd, &magic, sizeof(magic));
-write(fd, &header, sizeof(header));
+ret = qemu_write_full(fd, &magic, sizeof(magic));
+if (ret != sizeof(magic)) {
+ret = -errno;
+goto exit;
+}
+ret = qemu_write_full(fd, &header, sizeof(header));
+if (ret != sizeof(header)) {
+ret = -errno;
+goto exit;
+}
-ftruncate(fd, header.grain_offset << 9);
+ret = ftruncate(fd, header.grain_offset << 9);
+if (ret < 0) {
+ret = -errno;
+goto exit;
+}
/* write grain directory */
lseek(fd, le64_to_cpu(header.rgd_offset) << 9, SEEK_SET);
for (i = 0, tmp = header.rgd_offset + gd_size;
- i < gt_count; i++, tmp += gt_size)
-write(fd, &tmp, sizeof(tmp));
+ i < gt_count; i++, tmp += gt_size) {
+ret = qemu_write_full(fd, &tmp, sizeof(tmp));
+if (ret != sizeof(tmp)) {
+ret = -errno;
+goto exit;
+}
+}
/* write backup grain directory */
lseek(fd, le64_to_cpu(header.gd_offset) << 9, SEEK_SET);
for (i = 0, tmp = header.gd_offset + gd_size;
- i < gt_count; i++, tmp += gt_size)
-write(fd, &tmp, sizeof(tmp));
+ i < gt_count; i++, tmp += gt_size) {
+ret = qemu_write_full(fd, &tmp, sizeof(tmp));
+if (ret != sizeof(tmp)) {
+ret = -errno;
+goto exit;
+}
+}
/* compose the descriptor */
real_filename = filename;
@@ -804,10 +828,16 @@ static int vmdk_create(const char *filename,
QEMUOptionParameter *options)
/* write the descriptor */
lseek(fd, le64_to_cpu(header.desc_offset) << 9, SEEK_SET);
-write(fd, desc, strlen(desc));
+ret = qemu_write_full(fd, desc, strlen(desc));
+if (ret != strlen(desc)) {
+ret = -errno;
+goto exit;
+}
+ret = 0;
+exit:
close(fd);
-return 0;
+return ret;
}
static void vmdk_close(BlockDriverState *bs)
--
1.6.5.7
[Qemu-devel] [PATCH 06/14] block/vvfat.c: fix warnings with _FORTIFY_SOURCE
CCblock/vvfat.o
cc1: warnings being treated as errors
block/vvfat.c: In function 'commit_one_file':
block/vvfat.c:2259: error: ignoring return value of 'ftruncate', declared with
attribute warn_unused_result
make: *** [block/vvfat.o] Error 1
CCblock/vvfat.o
In file included from /usr/include/stdio.h:912,
from ./qemu-common.h:19,
from block/vvfat.c:27:
In function 'snprintf',
inlined from 'init_directories' at block/vvfat.c:871,
inlined from 'vvfat_open' at block/vvfat.c:1068:
/usr/include/bits/stdio2.h:65: error: call to __builtin___snprintf_chk will
always overflow destination buffer
make: *** [block/vvfat.o] Error 1
Signed-off-by: Kirill A. Shutemov
---
block/vvfat.c |8 ++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/block/vvfat.c b/block/vvfat.c
index 063f731..a7ec6b7 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -868,7 +868,8 @@ static int init_directories(BDRVVVFATState* s,
{
direntry_t* entry=array_get_next(&(s->directory));
entry->attributes=0x28; /* archive | volume label */
- snprintf((char*)entry->name,11,"QEMU VVFAT");
+ snprintf((char*)entry->name,8,"QEMU VV");
+ snprintf((char*)entry->extension,3,"FAT");
}
/* Now build FAT, and write back information into directory */
@@ -2256,7 +2257,10 @@ static int commit_one_file(BDRVVVFATState* s,
c = c1;
}
-ftruncate(fd, size);
+if (ftruncate(fd, size)) {
+ perror("ftruncate()");
+ abort();
+}
close(fd);
return commit_mappings(s, first_cluster, dir_index);
--
1.6.5.7
[Qemu-devel] [PATCH 07/14] block/qcow2.c: fix warnings with _FORTIFY_SOURCE
CCblock/qcow2.o
cc1: warnings being treated as errors
block/qcow2.c: In function 'qcow_create2':
block/qcow2.c:829: error: ignoring return value of 'write', declared with
attribute warn_unused_result
block/qcow2.c:838: error: ignoring return value of 'write', declared with
attribute warn_unused_result
block/qcow2.c:839: error: ignoring return value of 'write', declared with
attribute warn_unused_result
block/qcow2.c:841: error: ignoring return value of 'write', declared with
attribute warn_unused_result
block/qcow2.c:844: error: ignoring return value of 'write', declared with
attribute warn_unused_result
block/qcow2.c:849: error: ignoring return value of 'write', declared with
attribute warn_unused_result
block/qcow2.c:852: error: ignoring return value of 'write', declared with
attribute warn_unused_result
block/qcow2.c:855: error: ignoring return value of 'write', declared with
attribute warn_unused_result
make: *** [block/qcow2.o] Error 1
Signed-off-by: Kirill A. Shutemov
---
block/qcow2.c | 55 +--
1 files changed, 45 insertions(+), 10 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 984264b..44dc195 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -743,7 +743,7 @@ static int qcow_create2(const char *filename, int64_t
total_size,
uint64_t tmp, offset;
QCowCreateState s1, *s = &s1;
QCowExtension ext_bf = {0, 0};
-
+int ret;
memset(s, 0, sizeof(*s));
@@ -826,7 +826,11 @@ static int qcow_create2(const char *filename, int64_t
total_size,
ref_clusters * s->cluster_size);
/* write all the data */
-write(fd, &header, sizeof(header));
+ret = qemu_write_full(fd, &header, sizeof(header));
+if (ret != sizeof(header)) {
+ret = -errno;
+goto exit;
+}
if (backing_file) {
if (backing_format_len) {
char zero[16];
@@ -835,25 +839,56 @@ static int qcow_create2(const char *filename, int64_t
total_size,
memset(zero, 0, sizeof(zero));
cpu_to_be32s(&ext_bf.magic);
cpu_to_be32s(&ext_bf.len);
-write(fd, &ext_bf, sizeof(ext_bf));
-write(fd, backing_format, backing_format_len);
+ret = qemu_write_full(fd, &ext_bf, sizeof(ext_bf));
+if (ret != sizeof(ext_bf)) {
+ret = -errno;
+goto exit;
+}
+ret = qemu_write_full(fd, backing_format, backing_format_len);
+if (ret != backing_format_len) {
+ret = -errno;
+goto exit;
+}
if (padding > 0) {
-write(fd, zero, padding);
+ret = qemu_write_full(fd, zero, padding);
+if (ret != padding) {
+ret = -errno;
+goto exit;
+}
}
}
-write(fd, backing_file, backing_filename_len);
+ret = qemu_write_full(fd, backing_file, backing_filename_len);
+if (ret != backing_filename_len) {
+ret = -errno;
+goto exit;
+}
}
lseek(fd, s->l1_table_offset, SEEK_SET);
tmp = 0;
for(i = 0;i < l1_size; i++) {
-write(fd, &tmp, sizeof(tmp));
+ret = qemu_write_full(fd, &tmp, sizeof(tmp));
+if (ret != sizeof(tmp)) {
+ret = -errno;
+goto exit;
+}
}
lseek(fd, s->refcount_table_offset, SEEK_SET);
-write(fd, s->refcount_table, s->cluster_size);
+ret = qemu_write_full(fd, s->refcount_table, s->cluster_size);
+if (ret != s->cluster_size) {
+ret = -errno;
+goto exit;
+}
lseek(fd, s->refcount_block_offset, SEEK_SET);
-write(fd, s->refcount_block, ref_clusters * s->cluster_size);
+ret = qemu_write_full(fd, s->refcount_block,
+ ref_clusters * s->cluster_size);
+if (ret != s->cluster_size) {
+ret = -errno;
+goto exit;
+}
+ret = 0;
+exit:
qemu_free(s->refcount_table);
qemu_free(s->refcount_block);
close(fd);
@@ -867,7 +902,7 @@ static int qcow_create2(const char *filename, int64_t
total_size,
bdrv_close(bs);
}
-return 0;
+return ret;
}
static int qcow_create(const char *filename, QEMUOptionParameter *options)
--
1.6.5.7
[Qemu-devel] [PATCH 09/14] usb-linux.c: fix warning with _FORTIFY_SOURCE
CCusb-linux.o
cc1: warnings being treated as errors
usb-linux.c: In function 'usb_host_read_file':
usb-linux.c:1204: error: ignoring return value of 'fgets', declared with
attribute warn_unused_result
make: *** [usb-linux.o] Error 1
Signed-off-by: Kirill A. Shutemov
---
usb-linux.c |8 ++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/usb-linux.c b/usb-linux.c
index 88728e9..8673474 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1201,9 +1201,13 @@ static int usb_host_read_file(char *line, size_t
line_size, const char *device_f
device_file);
f = fopen(filename, "r");
if (f) {
-fgets(line, line_size, f);
+if (fgets(line, line_size, f)) {
+ret = 1;
+} else {
+ret = 0;
+}
+
fclose(f);
-ret = 1;
#if 0
} else {
if (mon)
--
1.6.5.7
[Qemu-devel] [PATCH 08/14] net/slirp.c: fix warning with _FORTIFY_SOURCE
CCnet/slirp.o
cc1: warnings being treated as errors
net/slirp.c: In function 'slirp_smb_cleanup':
net/slirp.c:470: error: ignoring return value of 'system', declared with
attribute warn_unused_result
make: *** [net/slirp.o] Error 1
Signed-off-by: Kirill A. Shutemov
---
net/slirp.c |9 -
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/net/slirp.c b/net/slirp.c
index 3f91c4b..2bd8a8f 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -464,10 +464,17 @@ int net_slirp_redir(const char *redir_str)
static void slirp_smb_cleanup(SlirpState *s)
{
char cmd[128];
+int ret;
if (s->smb_dir[0] != '\0') {
snprintf(cmd, sizeof(cmd), "rm -rf %s", s->smb_dir);
-system(cmd);
+ret = system(cmd);
+if (ret == -1) {
+perror("system()");
+} else if (WEXITSTATUS(ret)) {
+qemu_error("'%s' failed. Error code: %d\n"
+cmd, WEXITSTATUS(ret));
+}
s->smb_dir[0] = '\0';
}
}
--
1.6.5.7
[Qemu-devel] [PATCH 10/14] vl.c: fix warning with _FORTIFY_SOURCE
CCi386-softmmu/vl.o
cc1: warnings being treated as errors
/usr/src/RPM/BUILD/qemu-0.11.92/vl.c: In function 'qemu_event_increment':
/usr/src/RPM/BUILD/qemu-0.11.92/vl.c:3404: error: ignoring return value of
'write', declared with attribute warn_unused_result
/usr/src/RPM/BUILD/qemu-0.11.92/vl.c: In function 'main':
/usr/src/RPM/BUILD/qemu-0.11.92/vl.c:5774: error: ignoring return value of
'write', declared with attribute warn_unused_result
/usr/src/RPM/BUILD/qemu-0.11.92/vl.c:6064: error: ignoring return value of
'chdir', declared with attribute warn_unused_result
/usr/src/RPM/BUILD/qemu-0.11.92/vl.c:6083: error: ignoring return value of
'chdir', declared with attribute warn_unused_result
make[1]: *** [vl.o] Error 1
Signed-off-by: Kirill A. Shutemov
---
vl.c | 23 +++
1 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/vl.c b/vl.c
index e881e45..4527427 100644
--- a/vl.c
+++ b/vl.c
@@ -3390,11 +3390,17 @@ static int io_thread_fd = -1;
static void qemu_event_increment(void)
{
static const char byte = 0;
+int ret;
if (io_thread_fd == -1)
return;
-write(io_thread_fd, &byte, sizeof(byte));
+ret = write(io_thread_fd, &byte, sizeof(byte));
+if (ret < 0 && (errno != EINTR && errno != EAGAIN)) {
+fprintf(stderr, "qemu_event_increment: write() filed: %s\n",
+strerror(errno));
+exit (1);
+}
}
static void qemu_event_read(void *opaque)
@@ -5778,7 +5784,10 @@ int main(int argc, char **argv, char **envp)
#ifndef _WIN32
if (daemonize) {
uint8_t status = 1;
-write(fds[1], &status, 1);
+if (write(fds[1], &status, 1) != 1) {
+perror("write()");
+exit(1);
+}
} else
#endif
fprintf(stderr, "Could not acquire pid file: %s\n",
strerror(errno));
@@ -6075,7 +6084,10 @@ int main(int argc, char **argv, char **envp)
if (len != 1)
exit(1);
- chdir("/");
+ if (chdir("/")) {
+perror("chdir()");
+exit(1);
+}
TFR(fd = qemu_open("/dev/null", O_RDWR));
if (fd == -1)
exit(1);
@@ -6094,7 +6106,10 @@ int main(int argc, char **argv, char **envp)
fprintf(stderr, "chroot failed\n");
exit(1);
}
-chdir("/");
+if (chdir("/")) {
+perror("chdir()");
+exit(1);
+}
}
if (run_as) {
--
1.6.5.7
[Qemu-devel] [PATCH 11/14] monitor.c: fix warnings with _FORTIFY_SOURCE
CCi386-softmmu/monitor.o
cc1: warnings being treated as errors
/usr/src/RPM/BUILD/qemu-0.11.92/monitor.c: In function 'do_memory_save':
/usr/src/RPM/BUILD/qemu-0.11.92/monitor.c:1318: error: ignoring return value of
'fwrite', declared with attribute warn_unused_result
/usr/src/RPM/BUILD/qemu-0.11.92/monitor.c: In function
'do_physical_memory_save':
/usr/src/RPM/BUILD/qemu-0.11.92/monitor.c:1345: error: ignoring return value of
'fwrite', declared with attribute warn_unused_result
make[1]: *** [monitor.o] Error 1
Signed-off-by: Kirill A. Shutemov
---
monitor.c | 12 ++--
1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/monitor.c b/monitor.c
index c0dc48e..54c7323 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1320,10 +1320,14 @@ static void do_memory_save(Monitor *mon, const QDict
*qdict, QObject **ret_data)
if (l > size)
l = size;
cpu_memory_rw_debug(env, addr, buf, l, 0);
-fwrite(buf, 1, l, f);
+if (fwrite(buf, 1, l, f) != l) {
+monitor_printf(mon, "fwrite() failed\n");
+goto exit;
+}
addr += l;
size -= l;
}
+exit:
fclose(f);
}
@@ -1347,11 +1351,15 @@ static void do_physical_memory_save(Monitor *mon, const
QDict *qdict,
if (l > size)
l = size;
cpu_physical_memory_rw(addr, buf, l, 0);
-fwrite(buf, 1, l, f);
+if (fwrite(buf, 1, l, f) != l) {
+monitor_printf(mon, "fwrite() failed\n");
+goto exit;
+}
fflush(f);
addr += l;
size -= l;
}
+exit:
fclose(f);
}
--
1.6.5.7
[Qemu-devel] [PATCH 12/14] linux-user/mmap.c: fix warnings with _FORTIFY_SOURCE
CCi386-linux-user/mmap.o
cc1: warnings being treated as errors
/usr/src/RPM/BUILD/qemu-0.11.92/linux-user/mmap.c: In function 'mmap_frag':
/usr/src/RPM/BUILD/qemu-0.11.92/linux-user/mmap.c:253: error: ignoring return
value of 'pread', declared with attribute warn_unused_result
/usr/src/RPM/BUILD/qemu-0.11.92/linux-user/mmap.c: In function 'target_mmap':
/usr/src/RPM/BUILD/qemu-0.11.92/linux-user/mmap.c:477: error: ignoring return
value of 'pread', declared with attribute warn_unused_result
make[1]: *** [mmap.o] Error 1
Signed-off-by: Kirill A. Shutemov
---
linux-user/mmap.c |6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 144fb7c..e496c64 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -250,7 +250,8 @@ static int mmap_frag(abi_ulong real_start,
mprotect(host_start, qemu_host_page_size, prot1 | PROT_WRITE);
/* read the corresponding file data */
-pread(fd, g2h(start), end - start, offset);
+if (pread(fd, g2h(start), end - start, offset) == -1)
+return -errno;
/* put final protection */
if (prot_new != (prot1 | PROT_WRITE))
@@ -474,7 +475,8 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int
prot,
-1, 0);
if (retaddr == -1)
goto fail;
-pread(fd, g2h(start), len, offset);
+if (pread(fd, g2h(start), len, offset) == -1)
+return -errno;
if (!(prot & PROT_WRITE)) {
ret = target_mprotect(start, len, prot);
if (ret != 0) {
--
1.6.5.7
[Qemu-devel] [PATCH 13/14] Enable _FORTIFY_SOURCE=2
_FORTIFY_SOURCE is a Glibc feature which adds memory and string function protection. Signed-off-by: Kirill A. Shutemov --- configure |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/configure b/configure index 18aed43..0cdcdb3 100755 --- a/configure +++ b/configure @@ -97,7 +97,7 @@ CFLAGS="-g $CFLAGS" QEMU_CFLAGS="-Wall -Wundef -Wendif-labels -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS" QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS" QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS" -QEMU_CFLAGS="-U_FORTIFY_SOURCE $QEMU_CFLAGS" +QEMU_CFLAGS="-D_FORTIFY_SOURCE=2 $QEMU_CFLAGS" QEMU_CFLAGS="-I. -I\$(SRC_PATH) $QEMU_CFLAGS" LDFLAGS="-g $LDFLAGS" -- 1.6.5.7
[Qemu-devel] [PATCH 14/14] Add -fstack-protector-all to CFLAGS
-fstack-protector-all emit extra code to check for buffer overflows, such as stack smashing attacks. This is done by adding a guard variable to functions with vulnerable objects. Signed-off-by: Kirill A. Shutemov --- configure |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/configure b/configure index 0cdcdb3..16b70d8 100755 --- a/configure +++ b/configure @@ -98,6 +98,7 @@ QEMU_CFLAGS="-Wall -Wundef -Wendif-labels -Wwrite-strings -Wmissing-prototypes $ QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS" QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS" QEMU_CFLAGS="-D_FORTIFY_SOURCE=2 $QEMU_CFLAGS" +QEMU_CFLAGS="-fstack-protector-all $QEMU_CFLAGS" QEMU_CFLAGS="-I. -I\$(SRC_PATH) $QEMU_CFLAGS" LDFLAGS="-g $LDFLAGS" -- 1.6.5.7
Re: [Qemu-devel] [PATCH 01/14] Introduce qemu_write_full()
On Thu, 31 Dec 2009, Kirill A. Shutemov wrote:
> A variant of write(2) which handles partial write.
>
> Signed-off-by: Kirill A. Shutemov
> ---
> osdep.c | 27 +++
> qemu-common.h |1 +
> 2 files changed, 28 insertions(+), 0 deletions(-)
>
> diff --git a/osdep.c b/osdep.c
> index e4836e7..d2406f2 100644
> --- a/osdep.c
> +++ b/osdep.c
> @@ -243,6 +243,33 @@ int qemu_open(const char *name, int flags, ...)
> return ret;
> }
>
> +/*
> + * A variant of write(2) which handles partial write.
> + *
> + * Return the number of bytes transferred.
> + * Set errno if fewer than `count' bytes are written.
> + */
> +ssize_t qemu_write_full(int fd, const void *buf, size_t count)
> +{
> +ssize_t ret = 0;
> +ssize_t total = 0;
> +
> +while (count) {
> +ret = write(fd, buf, count);
> +if (ret < 0) {
> +if (errno == EINTR)
> +continue;
> +break;
> +}
> +
> +count -= ret;
> +buf += ret;
> + total += ret;
> +}
> +
> +return total;
> +}
This hides write errors.
> +
> #ifndef _WIN32
> /*
> * Creates a pipe with FD_CLOEXEC set on both file descriptors
> diff --git a/qemu-common.h b/qemu-common.h
> index 8630f8c..a8144cb 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -160,6 +160,7 @@ void qemu_mutex_lock_iothread(void);
> void qemu_mutex_unlock_iothread(void);
>
> int qemu_open(const char *name, int flags, ...);
> +ssize_t qemu_write_full(int fd, const void *buf, size_t count);
> void qemu_set_cloexec(int fd);
>
> #ifndef _WIN32
>
--
mailto:av1...@comtv.ru
Re: [Qemu-devel] [PATCH 04/14] block/qcow.c: fix warnings with _FORTIFY_SOURCE
On Thu, 31 Dec 2009, Kirill A. Shutemov wrote:
> CCblock/qcow.o
> cc1: warnings being treated as errors
> block/qcow.c: In function 'qcow_create':
> block/qcow.c:804: error: ignoring return value of 'write', declared with
> attribute warn_unused_result
> block/qcow.c:806: error: ignoring return value of 'write', declared with
> attribute warn_unused_result
> block/qcow.c:811: error: ignoring return value of 'write', declared with
> attribute warn_unused_result
> make: *** [block/qcow.o] Error 1
>
> Signed-off-by: Kirill A. Shutemov
> ---
> block/qcow.c | 26 ++
> 1 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/block/qcow.c b/block/qcow.c
> index 7fc85ae..472494c 100644
> --- a/block/qcow.c
> +++ b/block/qcow.c
> @@ -750,6 +750,7 @@ static int qcow_create(const char *filename,
> QEMUOptionParameter *options)
> int64_t total_size = 0;
> const char *backing_file = NULL;
> int flags = 0;
> +int ret;
>
> /* Read out options */
> while (options && options->name) {
> @@ -801,17 +802,34 @@ static int qcow_create(const char *filename,
> QEMUOptionParameter *options)
> }
>
> /* write all the data */
> -write(fd, &header, sizeof(header));
> +ret = qemu_write_full(fd, &header, sizeof(header));
> +if (ret != sizeof(header)) {
> +ret = -errno;
> +goto exit;
> +}
> +
> if (backing_file) {
> -write(fd, backing_file, backing_filename_len);
> +ret = qemu_write_full(fd, backing_file, backing_filename_len);
> +if (ret != backing_filename_len) {
> +ret = -errno;
> +goto exit;
> +}
> +
> }
> lseek(fd, header_size, SEEK_SET);
Surprising that lseek's return value is not marked warn_unused_result.
> tmp = 0;
> for(i = 0;i < l1_size; i++) {
> -write(fd, &tmp, sizeof(tmp));
> +ret = qemu_write_full(fd, &tmp, sizeof(tmp));
> +if (ret != sizeof(tmp)) {
> +ret = -errno;
> +goto exit;
> +}
> }
> +
> +ret = 0;
> +exit:
> close(fd);
> -return 0;
> +return ret;
> }
>
> static int qcow_make_empty(BlockDriverState *bs)
>
--
mailto:av1...@comtv.ru
Re: [Qemu-devel] [PATCH 10/14] vl.c: fix warning with _FORTIFY_SOURCE
On Thu, 31 Dec 2009, Kirill A. Shutemov wrote:
> CCi386-softmmu/vl.o
> cc1: warnings being treated as errors
> /usr/src/RPM/BUILD/qemu-0.11.92/vl.c: In function 'qemu_event_increment':
> /usr/src/RPM/BUILD/qemu-0.11.92/vl.c:3404: error: ignoring return value of
> 'write', declared with attribute warn_unused_result
> /usr/src/RPM/BUILD/qemu-0.11.92/vl.c: In function 'main':
> /usr/src/RPM/BUILD/qemu-0.11.92/vl.c:5774: error: ignoring return value of
> 'write', declared with attribute warn_unused_result
> /usr/src/RPM/BUILD/qemu-0.11.92/vl.c:6064: error: ignoring return value of
> 'chdir', declared with attribute warn_unused_result
> /usr/src/RPM/BUILD/qemu-0.11.92/vl.c:6083: error: ignoring return value of
> 'chdir', declared with attribute warn_unused_result
> make[1]: *** [vl.o] Error 1
>
> Signed-off-by: Kirill A. Shutemov
> ---
> vl.c | 23 +++
> 1 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index e881e45..4527427 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3390,11 +3390,17 @@ static int io_thread_fd = -1;
> static void qemu_event_increment(void)
> {
> static const char byte = 0;
> +int ret;
>
> if (io_thread_fd == -1)
> return;
>
> -write(io_thread_fd, &byte, sizeof(byte));
> +ret = write(io_thread_fd, &byte, sizeof(byte));
write returns ssize_t
> +if (ret < 0 && (errno != EINTR && errno != EAGAIN)) {
Elsewhere in the series you only check for EAGAIN.
> +fprintf(stderr, "qemu_event_increment: write() filed: %s\n",
> +strerror(errno));
> +exit (1);
> +}
> }
>
> static void qemu_event_read(void *opaque)
> @@ -5778,7 +5784,10 @@ int main(int argc, char **argv, char **envp)
> #ifndef _WIN32
> if (daemonize) {
> uint8_t status = 1;
> -write(fds[1], &status, 1);
> +if (write(fds[1], &status, 1) != 1) {
> +perror("write()");
> +exit(1);
> +}
> } else
> #endif
> fprintf(stderr, "Could not acquire pid file: %s\n",
> strerror(errno));
> @@ -6075,7 +6084,10 @@ int main(int argc, char **argv, char **envp)
> if (len != 1)
> exit(1);
>
> - chdir("/");
> + if (chdir("/")) {
> +perror("chdir()");
> +exit(1);
> +}
> TFR(fd = qemu_open("/dev/null", O_RDWR));
> if (fd == -1)
> exit(1);
> @@ -6094,7 +6106,10 @@ int main(int argc, char **argv, char **envp)
> fprintf(stderr, "chroot failed\n");
> exit(1);
> }
> -chdir("/");
> +if (chdir("/")) {
> +perror("chdir()");
> +exit(1);
> +}
> }
>
> if (run_as) {
Here and elsewhere the indentation is off.
--
mailto:av1...@comtv.ru
Re: [Qemu-devel] [PATCH] Add definitions for current cpu models..
john cooper wrote:
> {
> +.name = "Merom",
> +.level = 2,
> +.vendor1 = CPUID_VENDOR_INTEL_1,
> +.vendor2 = CPUID_VENDOR_INTEL_2,
> +.vendor3 = CPUID_VENDOR_INTEL_3,
> +.family = 6, /* P6 */
> +.model = 2,
> +.stepping = 3,
> +.features = PPRO_FEATURES |
> +/* these features are needed for Win64 and aren't fully implemented
> */
> +CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
> +/* this feature is needed for Solaris and isn't fully implemented */
> +CPUID_PSE36,
> +.ext_features = CPUID_EXT_SSE3, /* from qemu64 */
Isn't SSE3 a generic feature on these Intel CPUs, so this comment is
unnecessary?
Or is SSE3 not present on a real Merom? If so, wouldn't it be better to omit
it?
> +.ext2_features = (PPRO_FEATURES & 0x0183F3FF) |
Could we have a meaningful name for the magic number, please?
Maybe even a:
#define PPRO_EXT2_FEATURES (PPRO_FEATURES & PPRO_EXT2_MASK)
#define PPRO_EXT2_MASK (CPUID_... | CPUID_... | ...) /* Fill in. */
> +CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
> +.ext3_features = CPUID_EXT3_SVM, /* from qemu64 */
> +.xlevel = 0x800A,
> +.model_id = "Intel Merom Core 2",
> +},
Does this mean requesting an Intel Merom will give the guest AMD's SVM
capability? That's handy for virtualisation, but not an accurate CPU
model. It seems inappropriate to name it "Merom", with model_id
"Intel Merom Core 2", if it's adding extra qemu-specific capabilities.
I would think few guests are likely to need the nested-SVM capability,
so it could be omitted when "Merom" is requested, and added as an
additional feature on request from the command line, just like other
cpuid features can be added.
> +{
> +.name = "Penryn",
> +.level = 2,
> +.vendor1 = CPUID_VENDOR_INTEL_1,
> +.vendor2 = CPUID_VENDOR_INTEL_2,
> +.vendor3 = CPUID_VENDOR_INTEL_3,
> +.family = 6, /* P6 */
> +.model = 2,
> +.stepping = 3,
> +.features = PPRO_FEATURES |
> +/* these features are needed for Win64 and aren't fully implemented
> */
> +CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
> +/* this feature is needed for Solaris and isn't fully implemented */
> +CPUID_PSE36,
> +.ext_features = CPUID_EXT_SSE3 |
> +CPUID_EXT_CX16 | CPUID_EXT_SSSE3 | CPUID_EXT_SSE41,
> +.ext2_features = (PPRO_FEATURES & 0x0183F3FF) |
> +CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
> +.ext3_features = CPUID_EXT3_SVM,
> +.xlevel = 0x800A,
> +.model_id = "Intel Penryn Core 2",
> +},
Same comments as above for Merom about SVM and the PPRO_FEATURES mask.
You don't include the "from qemu64" comments this time. Is there a reason?
> +{
> +.name = "Nehalem",
> +.level = 2,
> +.vendor1 = CPUID_VENDOR_INTEL_1,
> +.vendor2 = CPUID_VENDOR_INTEL_2,
> +.vendor3 = CPUID_VENDOR_INTEL_3,
> +.family = 6, /* P6 */
> +.model = 2,
> +.stepping = 3,
> +.features = PPRO_FEATURES |
> +/* these features are needed for Win64 and aren't fully implemented
> */
> +CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
> +/* this feature is needed for Solaris and isn't fully implemented */
> +CPUID_PSE36,
> +.ext_features = CPUID_EXT_SSE3 |
> +CPUID_EXT_CX16 | CPUID_EXT_SSSE3 | CPUID_EXT_SSE41 |
> +CPUID_EXT_SSE42 | CPUID_EXT_POPCNT,
> +.ext2_features = (PPRO_FEATURES & 0x0183F3FF) |
> +CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
> +.ext3_features = CPUID_EXT3_SVM,
> +.xlevel = 0x800A,
> +.model_id = "Intel Nehalem Core i7",
> +},
Same as previous.
> +{
> +.name = "Opteron_G1",
> +.level = 5,
> +.vendor1 = CPUID_VENDOR_INTEL_1,
> +.vendor2 = CPUID_VENDOR_INTEL_2,
> +.vendor3 = CPUID_VENDOR_INTEL_3,
Someone else has already enquired - why Intel vendor id?
> +.family = 15,
> +.model = 6,
> +.stepping = 1,
> +.features = PPRO_FEATURES |
> +/* these features are needed for Win64 and aren't fully implemented
> */
> +CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
> +/* this feature is needed for Solaris and isn't fully implemented */
> +CPUID_PSE36,
> +.ext_features = CPUID_EXT_SSE3 | CPUID_EXT_MONITOR,
> +.ext2_features = (PPRO_FEATURES & 0x0183F3FF) |
> +CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
> +.ext3_features = CPUID_EXT3_SVM,
> +.xlevel = 0x8008,
> +.model_id = "AMD Opteron G1",
> +},
Why do the AMD models have CPUID_EXT_MONITOR but the Intel ones don't.
Is it correct for the CPU models? Even a lowly 32-bit Int
Re: [Qemu-devel] [PATCH] add "info ioapic" monitor command
On 12/31/2009 12:46 AM, Anthony Liguori wrote: I wasn't worried about that, only the increased code duplication. I wasn't thinking there would be an info ioapic command but rather a generic device-info command that would work with any qdev device. No code duplication (in fact, much, much less in the long term). Machine printing isn't going to work. The information needs to be formatted to be legible and bit fields decoded. Some field's interpretation may depend on the value of others. In some case machine printing of vmstate will show you the history of that devices live migration bug fixes in addition to its state. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain.
Re: [Qemu-devel] [PATCH 01/14] Introduce qemu_write_full()
On Thu, Dec 31, 2009 at 4:00 AM, malc wrote:
> On Thu, 31 Dec 2009, Kirill A. Shutemov wrote:
>
>> A variant of write(2) which handles partial write.
>>
>> Signed-off-by: Kirill A. Shutemov
>> ---
>> osdep.c | 27 +++
>> qemu-common.h | 1 +
>> 2 files changed, 28 insertions(+), 0 deletions(-)
>>
>> diff --git a/osdep.c b/osdep.c
>> index e4836e7..d2406f2 100644
>> --- a/osdep.c
>> +++ b/osdep.c
>> @@ -243,6 +243,33 @@ int qemu_open(const char *name, int flags, ...)
>> return ret;
>> }
>>
>> +/*
>> + * A variant of write(2) which handles partial write.
>> + *
>> + * Return the number of bytes transferred.
>> + * Set errno if fewer than `count' bytes are written.
>> + */
>> +ssize_t qemu_write_full(int fd, const void *buf, size_t count)
>> +{
>> + ssize_t ret = 0;
>> + ssize_t total = 0;
>> +
>> + while (count) {
>> + ret = write(fd, buf, count);
>> + if (ret < 0) {
>> + if (errno == EINTR)
>> + continue;
>> + break;
>> + }
>> +
>> + count -= ret;
>> + buf += ret;
>> + total += ret;
>> + }
>> +
>> + return total;
>> +}
>
> This hides write errors.
Why do you think so? Return value < count indicates error.
Re: [Qemu-devel] [PATCH 01/14] Introduce qemu_write_full()
On Thu, 31 Dec 2009, Kirill A. Shutemov wrote:
> On Thu, Dec 31, 2009 at 4:00 AM, malc wrote:
> > On Thu, 31 Dec 2009, Kirill A. Shutemov wrote:
> >
> >> A variant of write(2) which handles partial write.
> >>
> >> Signed-off-by: Kirill A. Shutemov
> >> ---
> >> osdep.c | 27 +++
> >> qemu-common.h | 1 +
> >> 2 files changed, 28 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/osdep.c b/osdep.c
> >> index e4836e7..d2406f2 100644
> >> --- a/osdep.c
> >> +++ b/osdep.c
> >> @@ -243,6 +243,33 @@ int qemu_open(const char *name, int flags, ...)
> >> return ret;
> >> }
> >>
> >> +/*
> >> + * A variant of write(2) which handles partial write.
> >> + *
> >> + * Return the number of bytes transferred.
> >> + * Set errno if fewer than `count' bytes are written.
> >> + */
> >> +ssize_t qemu_write_full(int fd, const void *buf, size_t count)
> >> +{
> >> + ssize_t ret = 0;
> >> + ssize_t total = 0;
> >> +
> >> + while (count) {
> >> + ret = write(fd, buf, count);
> >> + if (ret < 0) {
> >> + if (errno == EINTR)
> >> + continue;
> >> + break;
> >> + }
> >> +
> >> + count -= ret;
> >> + buf += ret;
> >> + total += ret;
> >> + }
> >> +
> >> + return total;
> >> +}
> >
> > This hides write errors.
>
> Why do you think so? Return value < count indicates error.
It does not indicate _which_ error it was.
--
mailto:av1...@comtv.ru
[Qemu-devel] Re: [PATCH 01/14] Introduce qemu_write_full()
On 12/31/2009 02:33 AM, Kirill A. Shutemov wrote: diff --git a/qemu-common.h b/qemu-common.h index 8630f8c..a8144cb 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -160,6 +160,7 @@ void qemu_mutex_lock_iothread(void); void qemu_mutex_unlock_iothread(void); int qemu_open(const char *name, int flags, ...); +ssize_t qemu_write_full(int fd, const void *buf, size_t count); void qemu_set_cloexec(int fd); #ifndef _WIN32 This should also use __attribute__ ((warn_unused_result)). Paolo
[Qemu-devel] Re: [PATCH 06/14] block/vvfat.c: fix warnings with _FORTIFY_SOURCE
On 12/31/2009 02:33 AM, Kirill A. Shutemov wrote: + snprintf((char*)entry->name,8,"QEMU VV"); + snprintf((char*)entry->extension,3,"FAT"); Wrong, the split should be "QEMU VVF" and "AT". Even better, change it to "QEMU VVF" and "AT " with a trailing space and use memcpy since it's not a NULL-terminated name. Paolo
