Re: [Qemu-devel] [PATCH 3/9] QMP: First half of the new argument checking code

2010-06-01 Thread Markus Armbruster
Luiz Capitulino  writes:

> This commit introduces the first half of qmp_check_client_args(),
> which is the new client argument checker.
>
> It's introduced on top of the existing code, so that there are
> no regressions during the transition.
>
> It works this way: the command's args_type field (from
> qemu-monitor.hx) is transformed into a qdict. Then we iterate
> over it checking whether each mandatory argument has been
> provided by the client.
>
> All comunication between the iterator and its caller is done
> via the new QMPArgCheckRes type.
>
> Next commit adds the second half of this work: type checking
> and invalid argument detection.
>
> Signed-off-by: Luiz Capitulino 
> ---
>  monitor.c |  107 
> +
>  1 files changed, 107 insertions(+), 0 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index bc3cc18..47a0da8 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4259,6 +4259,108 @@ static int invalid_qmp_mode(const Monitor *mon, const 
> char *cmd_name)
>  return (qmp_cmd_mode(mon) ? is_cap : !is_cap);
>  }
>  
> +typedef struct QMPArgCheckRes {
> +int result;
> +int skip;

skip is write-only in this patch.

> +QDict *qdict;
> +} QMPArgCheckRes;
> +
> +/*
> + * Check if client passed all mandatory args
> + */
> +static void check_mandatory_args(const char *cmd_arg_name,
> + QObject *obj, void *opaque)
> +{
> +QString *type;
> +QMPArgCheckRes *res = opaque;
> +
> +if (res->result < 0) {
> +/* report only the first error */
> +return;
> +}

This is a sign that the iterator needs a way to break the loop.

> +
> +type = qobject_to_qstring(obj);
> +assert(type != NULL);
> +
> +if (qstring_get_str(type)[0] == 'O') {
> +QemuOptsList *opts_list = qemu_find_opts(cmd_arg_name);
> +assert(opts_list);
> +res->result = check_opts(opts_list, res->qdict);

I doubt this is the right place for calling check_opts.

Right now, it's only implemented for QemuOptsList with empty desc.  A
more complete version is in my tree (blockdev needs it).  Looks like
this:

static int check_opts(QemuOptsList *opts_list, QDict *args)
{
QemuOptDesc *desc;
CmdArgs cmd_args;

for (desc = opts_list->desc; desc->name; desc++) {
cmd_args_init(&cmd_args);
cmd_args.optional = 1;
switch (desc->type) {
case QEMU_OPT_STRING:
cmd_args.type = 's';
break;
case QEMU_OPT_BOOL:
cmd_args.type = '-';
break;
case QEMU_OPT_NUMBER:
case QEMU_OPT_SIZE:
cmd_args.type = 'l';
break;
}
qstring_append(cmd_args.name, desc->name);
if (check_arg(&cmd_args, args) < 0) {
QDECREF(cmd_args.name);
return -1;
}
QDECREF(cmd_args.name);
}
return 0;
}

> +res->skip = 1;
> +} else if (qstring_get_str(type)[0] != '-' &&
> +   qstring_get_str(type)[1] != '?' &&
> +   !qdict_haskey(res->qdict, cmd_arg_name)) {
> +res->result = -1;
> +qerror_report(QERR_MISSING_PARAMETER, cmd_arg_name);
> +}
> +}
[...]



Re: [Qemu-devel] [PATCH] virtio-blk: assign a default serial number if none provided

2010-06-01 Thread Michael Tokarev

02.06.2010 05:48, Ryan Harper wrote:
[]

  hw/virtio-blk.c |3 +++
+if (strlen(s->sn) == 0) {


Just out of curiocity (not that it is wrong or inefficient):
why
  strlen(s->sn)
and not, say,
  !s->sn[0]
?

/mjt



[Qemu-devel] [PATCH v2] x86: svm: Always clear event_inj on vmexit

2010-06-01 Thread Jan Kiszka
Erik van der Kouwe wrote:
> Hi,
> 
>> We currently only clear SVM_EVTINJ_VALID after successful interrupt
>> delivery. This apparently does not match real hardware which clears the
>> whole event_inj field on every vmexit, including unsuccessful interrupt
>> delivery.
> 
> Thanks for the patch. It is a bit hard for me to test right now as I
> messed up my test setup, but I will do so ASAP and let you know.
> 
> However, I'm worried that this patch may introduce a new problem (I may
> be mistaken though). There is still this code to load the exit interrupt
> info:
> 
> stl_phys(env->vm_vmcb + offsetof(struct vmcb,
>   control.exit_int_info_err),
>   ldl_phys(env->vm_vmcb + offsetof(struct vmcb,
>   control.event_inj_err)));
> 
> Now that event_inj is no longer loaded, won't this mean that
> exit_int_info and exit_int_info_err also won't be loaded?

Sorry, can't follow this ATM. But maybe you mean this: there is indeed a
problem with removing the clearance of event_inj.invalid as it may be
later on transferred into exit_int_info. And if we succeed with
injecting the event, that field must not remaind valid.

OK, here is v2:

--->

From: Jan Kiszka 

We currently only clear SVM_EVTINJ_VALID after successful interrupt
delivery. This apparently does not match real hardware which clears the
whole event_inj field on every vmexit, including unsuccessful interrupt
delivery.

Reported-by: Erik van der Kouwe 
Signed-off-by: Jan Kiszka 
---
 target-i386/op_helper.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
index dcbdfe7..52e8910 100644
--- a/target-i386/op_helper.c
+++ b/target-i386/op_helper.c
@@ -5388,6 +5388,7 @@ void helper_vmexit(uint32_t exit_code, uint64_t 
exit_info_1)
  ldl_phys(env->vm_vmcb + offsetof(struct vmcb, 
control.event_inj)));
 stl_phys(env->vm_vmcb + offsetof(struct vmcb, control.exit_int_info_err),
  ldl_phys(env->vm_vmcb + offsetof(struct vmcb, 
control.event_inj_err)));
+stl_phys(env->vm_vmcb + offsetof(struct vmcb, control.event_inj), 0);
 
 env->hflags2 &= ~HF2_GIF_MASK;
 /* FIXME: Resets the current ASID register to zero (host ASID). */
-- 
1.6.0.2



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] Re: [SeaBIOS] SMBIOS strings

2010-06-01 Thread Jes Sorensen
On 06/01/10 22:26, Sebastian Herbszt wrote:
> Jes Sorensen wrote:
>> Handle 0x0401, DMI type 4, 32 bytes
>> Processor Information
>> -   Socket Designation: CPU 1
>> +   Socket Designation: CPU01
> 
> smbios.c got
> snprintf((char*)start, 6, "CPU%2x", cpu_number);
> 
> It should print "CPU 1" instead of "CPU01" because the
> padding should be done with spaces not zeros. Maybe
> bvprintf() doesn't handle it correctly?

I looked at the man page for snprintf() and it isn't clear to me that it
is required to space pad when printing hex numbers.

Having looked at the other pieces, I think this is probably the only one
we might want to change. It should be pretty easy to just do something like:

if (cpu_number < 0x10)
snprintf("CPU %x", cpu_number);
else
snprintf("CPU%2x", cpu_number);

Esthetically I think this would be prettier, but question is whether
it's something to worry about or not.

Cheers,
Jes




Re: [Qemu-devel] [PATCH 1/9] QDict: Introduce qdict_get_try_bool()

2010-06-01 Thread Markus Armbruster
Luiz Capitulino  writes:

> Signed-off-by: Luiz Capitulino 
> ---
>  qdict.c |   18 ++
>  qdict.h |1 +
>  2 files changed, 19 insertions(+), 0 deletions(-)
>
> diff --git a/qdict.c b/qdict.c
> index 175bc17..ca3c3b1 100644
> --- a/qdict.c
> +++ b/qdict.c
> @@ -287,6 +287,24 @@ int64_t qdict_get_try_int(const QDict *qdict, const char 
> *key,
>  }
>  
>  /**
> + * qdict_get_try_bool(): Try to get a bool mapped by 'key'
> + *
> + * Return bool mapped by 'key', if it is not present in the
> + * dictionary or if the stored object is not of QBool type
> + * 'err_value' will be returned.
> + */
> +int qdict_get_try_bool(const QDict *qdict, const char *key, int err_value)
> +{
> +QObject *obj;
> +
> +obj = qdict_get(qdict, key);
> +if (!obj || qobject_type(obj) != QTYPE_QBOOL)
> +return err_value;
> +
> +return qbool_get_int(qobject_to_qbool(obj));
> +}
> +
> +/**
>   * qdict_get_try_str(): Try to get a pointer to the stored string
>   * mapped by 'key'
>   *
> diff --git a/qdict.h b/qdict.h
> index 5e5902c..5cca816 100644
> --- a/qdict.h
> +++ b/qdict.h
> @@ -57,6 +57,7 @@ QDict *qdict_get_qdict(const QDict *qdict, const char *key);
>  const char *qdict_get_str(const QDict *qdict, const char *key);
>  int64_t qdict_get_try_int(const QDict *qdict, const char *key,
>int64_t err_value);
> +int qdict_get_try_bool(const QDict *qdict, const char *key, int err_value);
>  const char *qdict_get_try_str(const QDict *qdict, const char *key);
>  
>  #endif /* QDICT_H */

Nitpick: there's precedence for parameter name "err_value" in qdict.c,
but it's a misleading name all the same.  The parameter is a default
value.  Missing key is *not* an error.



Re: [Qemu-devel] [PATCH 2/8] sparc64: fix missing address masking

2010-06-01 Thread Igor Kovalenko
On Wed, Jun 2, 2010 at 12:44 AM, Richard Henderson  wrote:
> On 06/01/2010 01:12 PM, Igor V. Kovalenko wrote:
>> +    if ((env->pstate & PS_AM) && is_translating_asi(asi)) {
>> +        addr &= 0xULL;
>> +    }
>
> I suggest that these be written instead as
>
>  if (is_translating_asi(asi)) {
>    addr = address_mask(addr);
>  }
>
> That should allow you to remove some of the ifdefs.

All address masking is done for sparc64 target only, sparc32 does not
have the notion of translating asi.
I think it's better to do debug printf macro trick then but I see no
real benefit at the moment.

-- 
Kind regards,
Igor V. Kovalenko



[Qemu-devel] [PATCH] Extra scan codes for missing keys

2010-06-01 Thread Bernhard M. Wiedemann
The code comes from
http://lists.gnu.org/archive/html/qemu-devel/2010-05/msg02788.html

Without this patch it is not possible to send at least 10 special
characters (\|'"`~:;[]{}) via the monitor sendkey command.

Signed-off-by: Bernhard M. Wiedemann 
---
 monitor.c |7 ++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index 15b53b9..1635040 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1563,7 +1563,8 @@ static const KeyDef key_defs[] = {
 { 0x17, "i" },
 { 0x18, "o" },
 { 0x19, "p" },
-
+{ 0x1a, "bracket_left" },
+{ 0x1b, "bracket_right" },
 { 0x1c, "ret" },
 
 { 0x1e, "a" },
@@ -1575,7 +1576,11 @@ static const KeyDef key_defs[] = {
 { 0x24, "j" },
 { 0x25, "k" },
 { 0x26, "l" },
+{ 0x27, "semicolon" },
+{ 0x28, "apostrophe" },
+{ 0x29, "grave_accent" },
 
+{ 0x2b, "backslash" },
 { 0x2c, "z" },
 { 0x2d, "x" },
 { 0x2e, "c" },
-- 
1.5.6.5



Re: [Qemu-devel] [PATCH 0/4] Add virtio disk identification support

2010-06-01 Thread john cooper
Ryan Harper wrote:
> I've applied the qemu and kernel side of this patch series and tested
> this out using the sample code below.  I've also reworked this example
> into a virtioblk_id tool to work with udev to generate /dev/disk/by-id
> links; I'll be submitting a patch set to linux-hotplug with these
> changes, (and update for path_id) and some udev rules to
> persistent-storage script to autogenerate by-id and by-path symlinks for
> virtio-blk devices.

Sorry I didn't respond to you earlier.  Actually the
guest visible interface below was only intended as an
example while waiting for (what I misinterpreted as
Marc's intention to create) a /sys interface.

I'm all for putting this issue to rest, but if we're
going to live with an ioctl interface retrieving the
id string, let's make it a little more friendly from
the user's perspective.  I have a slightly modified
version which basically implements the same interface
but expects the sizeof the entire array to be preset
in the first element.  Otherwise the user has no
way of informing the driver of the destination's size,
nor a way for the driver to indicate when the data
won't fit in the area specified by the user.

I should still have the patch on a test machine which
ATM is unaccessible, but will have at first thing tomorrow.
Let's hold off until then so we can address this nit
and avoid yet another hiccup in nailing down this
interface.

Thanks,

-john

> I've also got a patch to apply ontop of the qemu patches to generate a
> default serial number if one isn't specified (like we do for ide
> devices).
> 
> Anthony, is this series in your queue yet?
> 
> Acked-by: Ryan Harper 
> 
>>
>>
>> #include 
>> #include 
>> #include 
>> #include 
>> #include 
>>
>> #define IOCTL_CMD'VBID'
>>
>> main()
>> {
>>  int fd, rv;
>>  char buf[512];
>>
>>  bzero(buf, sizeof (buf));
>>  if ((fd = open("/dev/vda", O_RDONLY)) < 0)
>>  perror("open");
>>  else if (ioctl(fd, IOCTL_CMD, buf) < 0)
>>  perror("ioctl");
>>  else
>>  printf("[%s]\n", buf);
>> }
>>
>> -- 
>> john.coo...@redhat.com
>>
> 


-- 
john.coo...@redhat.com



[Qemu-devel] [PATCH] virtio-blk: assign a default serial number if none provided

2010-06-01 Thread Ryan Harper
This patch applies on-top of John's virtio-blk serial patches.

Generate default serial numbers for virtio drives based on DriveInfo.unit which 
is
incremented for each additional virtio-blk device.  This provides a
per-virtio-blk number to use in the default string: QM%05d that is used in
hw/ide/core.c.  The resulting serial number looks like: QM1, etc.

Signed-off-by: Ryan Harper 
---
 hw/virtio-blk.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 98c62f2..e5c6e7c 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -518,6 +518,9 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf 
*conf)
 bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs);
 
 strncpy(s->sn, drive_get_serial(s->bs), sizeof (s->sn));
+if (strlen(s->sn) == 0) {
+snprintf(s->sn, sizeof(s->sn), "QM%05d", conf->dinfo->unit);
+}
 
 s->vq = virtio_add_queue(&s->vdev, 128, virtio_blk_handle_output);
 
-- 
1.6.3.3


-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ry...@us.ibm.com



Re: [Qemu-devel] [PATCH 0/4] Add virtio disk identification support

2010-06-01 Thread Ryan Harper
* john cooper  [2010-03-25 00:45]:
> This series adds the minimal support to qemu and virtio_blk
> to support passing of a virtio_blk serial id string from qemu
> through the guest driver and to the guest userland.
> 
> This is derived in part from a patch set posted by Rusty some
> time ago, but has been minimized to remove support for prior
> versions which attempted to provide the same functionality via
> pci config/io space.  This version rather uses a virtio request
> as proposed in Rusty's example.
> 
> Also removed is the packaging of the serial/id string within
> the glorious bag of bits returned by the ATA_IDENTIFY command.
> Here we transfer only the 20 bytes of serial/id string from
> qemu to the guest userland.  In the proposed interface, this
> is made available by an ioctl() into the virtio_blk driver
> however other interfaces (eg: /sys) have also been proposed.
> A code snippet is attached below as an example of ioctl usage.
> 
> The resulting code is quite minimal and I believe it addresses
> all concerns raised in prior versions.
> 
> -john

I've applied the qemu and kernel side of this patch series and tested
this out using the sample code below.  I've also reworked this example
into a virtioblk_id tool to work with udev to generate /dev/disk/by-id
links; I'll be submitting a patch set to linux-hotplug with these
changes, (and update for path_id) and some udev rules to
persistent-storage script to autogenerate by-id and by-path symlinks for
virtio-blk devices.

I've also got a patch to apply ontop of the qemu patches to generate a
default serial number if one isn't specified (like we do for ide
devices).

Anthony, is this series in your queue yet?

Acked-by: Ryan Harper 

> 
> 
> 
> #include 
> #include 
> #include 
> #include 
> #include 
> 
> #define IOCTL_CMD 'VBID'
> 
> main()
> {
>   int fd, rv;
>   char buf[512];
> 
>   bzero(buf, sizeof (buf));
>   if ((fd = open("/dev/vda", O_RDONLY)) < 0)
>   perror("open");
>   else if (ioctl(fd, IOCTL_CMD, buf) < 0)
>   perror("ioctl");
>   else
>   printf("[%s]\n", buf);
> }
> 
> -- 
> john.coo...@redhat.com
> 

-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ry...@us.ibm.com



[Qemu-devel] [Bug 588127] Re: qemu fails to recognize host features SSE4.1, SSE4.2

2010-06-01 Thread fonz
Ok, building from source now:

Passing SSE4.* to the guest does not work for me thru qemu-kvm-0.12.4,
but it _does_ work at current head (git:aa22e82, 0.12.50).  I can bisect
further if anybody cares...

-- 
qemu fails to recognize host features SSE4.1, SSE4.2
https://bugs.launchpad.net/bugs/588127
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.

Status in QEMU: New

Bug description:
Tested on qemu v0.11.0 and v0.12.3:

>qemu-kvm -cpu core2duo,+sse4.1,+sse4.2
CPU feature sse4.1 not found
CPU feature sse4.2 not found

>egrep "model name|flags" /proc/cpuinfo | sort -r | uniq
model name  : Intel(R) Xeon(R) CPU   X5570  @ 2.93GHz
flags   : fpu vme de pse tsc msr pae mce cx8 apic mtrr pge mca cmov pat 
pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx rdtscp lm 
constant_tsc arch_perfmon pebs bts rep_good xtopology tsc_reliable nonstop_tsc 
pni dtes64 monitor ds_cpl vmx est tm2 ssse3 cx16 xtpr pdcm dca sse4_1 sse4_2 
popcnt lahf_lm tpr_shadow vnmi flexpriority ept vpid





Re: [SeaBIOS] [Qemu-devel] Re: SMBIOS strings

2010-06-01 Thread Kevin O'Connor
On Tue, Jun 01, 2010 at 08:05:26AM +0200, Jes Sorensen wrote:
> On 06/01/10 07:34, Markus Armbruster wrote:
> > "Sebastian Herbszt"  writes:
> >> Gleb Natapov wrote:
> >>> I don't care much as long as we will not have "CPU :". It looks like 
> >>> something
> >>> that can change after BIOS upgrade, so it is hard to believe Windows
> >>> will stop working because of this change.
> >> Maybe it could trigger the Windows activation process?
> > Isn't that testable?
> The problem there is that the number of possible combinations to test is
> endless. I think older versions of windows are far more prone to such
> problems than newer ones.

Lets select a handful of OS versions that we think are important and
then test.  If the smbios changes cause a problem, then we can
document that fact and be careful going forward.

I'm leery about reverting improvements on the premise that some guest
could possibly observe the change and be finicky about it.  That way
leads to stagnation.  (For example, for all I know, the recent smbios
memory layout bug fix could cause a Windows activation.)

BTW, is there a known issue with activation on the latest qemu?

-Kevin



[Qemu-devel] [PATCH] Fix multiboot compilation

2010-06-01 Thread Alexander Graf
Commit dd4239d6574ca41c94fc0d0f77ddc728510ffc57 broke multiboot. It replaced the
instruction "rep insb (%dx), %es:(%edi)" by the binary output of
"addr32 rep insb (%dx), %es:(%di)".

Linuxboot calls the respective helper function in a code16 section. So the
original instruction was automatically translated to its "addr32" equivalent.
For multiboot, we're running in code32 so gcc didn't add the "addr32" which
breaks the instruction.

This patch splits that helper function in one which uses addr32 and one which
does not, so everyone's happy.

The good news is that nobody probably cared so far. The bundled multiboot.bin
binary was built before the change and is thus correct.

Please also put this patch into -stable.

Signed-off-by: Alexander Graf 
---
 pc-bios/optionrom/linuxboot.S |8 
 pc-bios/optionrom/optionrom.h |   32 
 2 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/pc-bios/optionrom/linuxboot.S b/pc-bios/optionrom/linuxboot.S
index 8aebe51..c109363 100644
--- a/pc-bios/optionrom/linuxboot.S
+++ b/pc-bios/optionrom/linuxboot.S
@@ -106,10 +106,10 @@ copy_kernel:
/* We're now running in 16-bit CS, but 32-bit ES! */
 
/* Load kernel and initrd */
-   read_fw_blob(FW_CFG_KERNEL)
-   read_fw_blob(FW_CFG_INITRD)
-   read_fw_blob(FW_CFG_CMDLINE)
-   read_fw_blob(FW_CFG_SETUP)
+   read_fw_blob_addr32(FW_CFG_KERNEL)
+   read_fw_blob_addr32(FW_CFG_INITRD)
+   read_fw_blob_addr32(FW_CFG_CMDLINE)
+   read_fw_blob_addr32(FW_CFG_SETUP)
 
/* And now jump into Linux! */
mov $0, %eax
diff --git a/pc-bios/optionrom/optionrom.h b/pc-bios/optionrom/optionrom.h
index 4dcb906..fbdd48a 100644
--- a/pc-bios/optionrom/optionrom.h
+++ b/pc-bios/optionrom/optionrom.h
@@ -50,13 +50,7 @@
bswap   %eax
 .endm
 
-/*
- * Read a blob from the fw_cfg device.
- * Requires _ADDR, _SIZE and _DATA values for the parameter.
- *
- * Clobbers:   %eax, %edx, %es, %ecx, %edi
- */
-#define read_fw_blob(var)  \
+#define read_fw_blob_pre(var)  \
read_fw var ## _ADDR;   \
mov %eax, %edi; \
read_fw var ## _SIZE;   \
@@ -65,10 +59,32 @@
mov $BIOS_CFG_IOPORT_CFG, %edx; \
outw%ax, (%dx); \
mov $BIOS_CFG_IOPORT_DATA, %dx; \
-   cld;\
+   cld
+
+/*
+ * Read a blob from the fw_cfg device.
+ * Requires _ADDR, _SIZE and _DATA values for the parameter.
+ *
+ * Clobbers:   %eax, %edx, %es, %ecx, %edi
+ */
+#define read_fw_blob(var)  \
+   read_fw_blob_pre(var);  \
/* old as(1) doesn't like this insn so emit the bytes instead: \
rep insb(%dx), %es:(%edi);  \
*/  \
+   .dc.b   0xf3,0x6c
+
+/*
+ * Read a blob from the fw_cfg device in forced addr32 mode.
+ * Requires _ADDR, _SIZE and _DATA values for the parameter.
+ *
+ * Clobbers:   %eax, %edx, %es, %ecx, %edi
+ */
+#define read_fw_blob_addr32(var)   \
+   read_fw_blob_pre(var);  \
+   /* old as(1) doesn't like this insn so emit the bytes instead: \
+   addr32 rep insb (%dx), %es:(%edi);  \
+   */  \
.dc.b   0x67,0xf3,0x6c
 
 #define OPTION_ROM_START   \
-- 
1.6.0.2




[Qemu-devel] Re: [SeaBIOS] SMBIOS strings

2010-06-01 Thread Kevin O'Connor
On Tue, Jun 01, 2010 at 10:26:12PM +0200, Sebastian Herbszt wrote:
> Jes Sorensen wrote:
> >Handle 0x0401, DMI type 4, 32 bytes
> >Processor Information
> >-   Socket Designation: CPU 1
> >+   Socket Designation: CPU01
> 
> smbios.c got
> snprintf((char*)start, 6, "CPU%2x", cpu_number);
> 
> It should print "CPU 1" instead of "CPU01" because the
> padding should be done with spaces not zeros. Maybe
> bvprintf() doesn't handle it correctly?

Space padding hasn't been implemented - nothing needed it.

The bvprintf code is called from 16bit code which is very stack
sensitive - if space padding is implemented it will have to be tested
carefully.

-Kevin



[Qemu-devel] [PATCH] pc: push setting default cpu_model down a level

2010-06-01 Thread Alex Williamson
Not that CPU hotplug currently works, but if you make the mistake of
trying it on a VM started without specifying a -cpu value, you hit
a segfault from trying to strdup(NULL) in cpu_x86_find_by_name().

Signed-off-by: Alex Williamson 
---

 hw/pc.c |   16 
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 9b85c42..a79586c 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -771,6 +771,14 @@ static CPUState *pc_new_cpu(const char *cpu_model)
 {
 CPUState *env;
 
+if (cpu_model == NULL) {
+#ifdef TARGET_X86_64
+cpu_model = "qemu64";
+#else
+cpu_model = "qemu32";
+#endif
+}
+
 env = cpu_init(cpu_model);
 if (!env) {
 fprintf(stderr, "Unable to find x86 CPU definition\n");
@@ -791,14 +799,6 @@ void pc_cpus_init(const char *cpu_model)
 int i;
 
 /* init CPUs */
-if (cpu_model == NULL) {
-#ifdef TARGET_X86_64
-cpu_model = "qemu64";
-#else
-cpu_model = "qemu32";
-#endif
-}
-
 for(i = 0; i < smp_cpus; i++) {
 pc_new_cpu(cpu_model);
 }




[Qemu-devel] [PATCH] monitor: allow device to be ejected if no disk is inserted

2010-06-01 Thread Eduardo Habkost
Resubmitting a patch that was submitted in December[1]. It was on the staging
tree but somehow it got dropped. I have rebased it to current master branch on
git.

  [1] http://article.gmane.org/gmane.comp.emulators.qemu/59813



This changes the monitor eject_device() function to not check for
bdrv_is_inserted().

Example run where the bug manifests itself:

(output of 'info block' is stripped to include only the CD-ROM device)

  (qemu) info block
  ide1-cd0: type=cdrom removable=1 locked=0 [not inserted]
  (qemu) change ide1-cd0 /dev/cdrom host_cdrom
  (qemu) info block
  ide1-cd0: type=cdrom removable=1 locked=0 file=/dev/cdrom ro=1 drv=host_cdrom 
encrypted=0
  (qemu) eject ide1-cd0
  (qemu) info block
  ide1-cd0: type=cdrom removable=1 locked=0 file=/dev/cdrom ro=1 drv=host_cdrom 
encrypted=0

  # at this point, a disk was inserted on the host CD-ROM drive

  (qemu) info block
  ide1-cd0: type=cdrom removable=1 locked=0 file=/dev/cdrom ro=1 drv=host_cdrom 
encrypted=0
  (qemu) eject ide1-cd0
  (qemu) info block
  ide1-cd0: type=cdrom removable=1 locked=0 [not inserted]
  (qemu)

The first eject command didn't work because the is_inserted() check
failed.

I have no clue why the code had the is_inserted() check, as it doesn't matter
if there is a disk present at the host drive, when the user wants the virtual
device to be disconnected from the host device.

The is_inserted() check has another side effect: a memory leak if the "change"
command is used multiple times, as do_change() calls eject_device() before
re-opening the block device, but bdrv_close() is never called.

Signed-off-by: Eduardo Habkost 
---
 monitor.c |   22 ++
 1 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/monitor.c b/monitor.c
index 15b53b9..57c355a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -951,20 +951,18 @@ static int do_quit(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
 
 static int eject_device(Monitor *mon, BlockDriverState *bs, int force)
 {
-if (bdrv_is_inserted(bs)) {
-if (!force) {
-if (!bdrv_is_removable(bs)) {
-qerror_report(QERR_DEVICE_NOT_REMOVABLE,
-   bdrv_get_device_name(bs));
-return -1;
-}
-if (bdrv_is_locked(bs)) {
-qerror_report(QERR_DEVICE_LOCKED, bdrv_get_device_name(bs));
-return -1;
-}
+if (!force) {
+if (!bdrv_is_removable(bs)) {
+qerror_report(QERR_DEVICE_NOT_REMOVABLE,
+   bdrv_get_device_name(bs));
+return -1;
+}
+if (bdrv_is_locked(bs)) {
+qerror_report(QERR_DEVICE_LOCKED, bdrv_get_device_name(bs));
+return -1;
 }
-bdrv_close(bs);
 }
+bdrv_close(bs);
 return 0;
 }
 
-- 
1.7.0.3



Re: [Qemu-devel] [PATCH 1/3] vnc: tight: don't forget last pixel in tight_encode_indexed_rect

2010-06-01 Thread Anthony Liguori

On 06/01/2010 04:05 PM, Corentin Chary wrote:

A simple patch would have been to just remove count -= 1, but this
one also replace the while (count--) with a for(i = 0; i<  count; i++)
which I believe is more easy to understand.

Signed-off-by: Corentin Chary
   


Applied all.  Thanks.

Regards,

Anthony Liguori


---
  vnc-encoding-tight.c |9 -
  1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/vnc-encoding-tight.c b/vnc-encoding-tight.c
index 50be44e..c8effe6 100644
--- a/vnc-encoding-tight.c
+++ b/vnc-encoding-tight.c
@@ -249,18 +249,16 @@ static void print_palette(const char *key, QObject *obj, 
void *opaque)
  uint##bpp##_t *src; \
  uint##bpp##_t rgb;  \
  uint8_t key[6]; \
-int rep = 0;\
+int i, rep; \
  uint8_t idx;\
  \
  src = (uint##bpp##_t *) buf;\
  \
-count -= 1; \
-while (count--) {   \
+for (i = 0; i<  count; i++) {   \
  rgb = *src++;   \
  rep = 0;\
-while (count&&  *src == rgb) {  \
-rep++, src++, count--;  \
+while (i<  count&&  *src == rgb) {  \
+rep++, src++, i++;  \
  }   \
  tight_palette_rgb2buf(rgb, bpp, key);   \
  if (!qdict_haskey(palette, (char *)key)) {  \
   





[Qemu-devel] [PATCH 3/3] vnc: add missing target for vnc-encodings-*.o

2010-06-01 Thread Corentin Chary
vnc-encodings-*.c dependencies where missing.

Signed-off-by: Corentin Chary 
---
 Makefile |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index cc5fc45..221fbd8 100644
--- a/Makefile
+++ b/Makefile
@@ -120,11 +120,11 @@ vnc-auth-vencrypt.o: vnc-auth-vencrypt.c vnc.h
 
 vnc-auth-sasl.o: vnc-auth-sasl.c vnc.h
 
-vnc-encoding-zlib.o: vnc.h
+vnc-encoding-zlib.o: vnc-encoding-zlib.c vnc.h
 
-vnc-encoding-hextile.o: vnc.h
+vnc-encoding-hextile.o: vnc-encoding-hextile.c vnc.h
 
-vnc-encoding-tight.o: vnc.h vnc-encoding-tight.h
+vnc-encoding-tight.o: vnc-encoding-tight.c vnc.h vnc-encoding-tight.h
 
 curses.o: curses.c keymaps.h curses_keys.h
 
-- 
1.7.1




[Qemu-devel] [PATCH 2/3] vnc: tight: don't forget the third color

2010-06-01 Thread Corentin Chary
While couting color, if the third color was only present one
time it wasn't added to the palette.

Signed-off-by: Corentin Chary 
---
 vnc-encoding-tight.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/vnc-encoding-tight.c b/vnc-encoding-tight.c
index c8effe6..efb57e7 100644
--- a/vnc-encoding-tight.c
+++ b/vnc-encoding-tight.c
@@ -177,6 +177,7 @@ static int tight_palette_insert(QDict *palette, uint32_t 
rgb, int bpp, int max)
 *palette = qdict_new(); \
 tight_palette_insert(*palette, c0, bpp, max);   \
 tight_palette_insert(*palette, c1, bpp, max);   \
+tight_palette_insert(*palette, ci, bpp, max);   \
 \
 for (i++; i < count; i++) { \
 if (data[i] == ci) {\
-- 
1.7.1




[Qemu-devel] [PATCH 1/3] vnc: tight: don't forget last pixel in tight_encode_indexed_rect

2010-06-01 Thread Corentin Chary
A simple patch would have been to just remove count -= 1, but this
one also replace the while (count--) with a for(i = 0; i < count; i++)
which I believe is more easy to understand.

Signed-off-by: Corentin Chary 
---
 vnc-encoding-tight.c |9 -
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/vnc-encoding-tight.c b/vnc-encoding-tight.c
index 50be44e..c8effe6 100644
--- a/vnc-encoding-tight.c
+++ b/vnc-encoding-tight.c
@@ -249,18 +249,16 @@ static void print_palette(const char *key, QObject *obj, 
void *opaque)
 uint##bpp##_t *src; \
 uint##bpp##_t rgb;  \
 uint8_t key[6]; \
-int rep = 0;\
+int i, rep; \
 uint8_t idx;\
 \
 src = (uint##bpp##_t *) buf;\
 \
-count -= 1; \
-while (count--) {   \
+for (i = 0; i < count; i++) {   \
 rgb = *src++;   \
 rep = 0;\
-while (count && *src == rgb) {  \
-rep++, src++, count--;  \
+while (i < count && *src == rgb) {  \
+rep++, src++, i++;  \
 }   \
 tight_palette_rgb2buf(rgb, bpp, key);   \
 if (!qdict_haskey(palette, (char *)key)) {  \
-- 
1.7.1




[Qemu-devel] [PATCH 0/3] Small tight fixes

2010-06-01 Thread Corentin Chary
Hi,
Here is two small tight fix and another small patch related to vnc encodings.
Thanks,

Corentin Chary (3):
  vnc: tight: don't forget last pixel in tight_encode_indexed_rect
  vnc: tight: don't forget the third color
  vnc: add missing target for vnc-encodings-*.o

 Makefile |6 +++---
 vnc-encoding-tight.c |   10 +-
 2 files changed, 8 insertions(+), 8 deletions(-)




Re: [Qemu-devel] [PATCH 1/3] vnc: tight: don't forget last pixel in tight_encode_indexed_rect

2010-06-01 Thread Corentin Chary
On Thu, May 27, 2010 at 4:28 PM, Richard Henderson  wrote:
> On 05/26/2010 11:21 PM, Corentin Chary wrote:
>> -        int rep = 0;                                                    \
>> +        int i = 0, rep = 0;                                             \
>
> Dead initialization.
>
>
> r~
>

Good catch, I'll re-send this set.


-- 
Corentin Chary
http://xf.iksaif.net



[Qemu-devel] [PATCH 4/9] QMP: Second half of the new argument checking code

2010-06-01 Thread Luiz Capitulino
This commit introduces check_client_args_type(), which is
called by qmp_check_client_args() and complements the
previous commit.

Now the new client's argument checker code is capable of
doing type checking and detecting unknown arguments.

It works this way: we iterate over the client's arguments
qdict and for each argument we check if it exists and if
its type is correct.

Signed-off-by: Luiz Capitulino 
---
 monitor.c |   77 -
 1 files changed, 76 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index 47a0da8..14790e6 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4266,6 +4266,75 @@ typedef struct QMPArgCheckRes {
 } QMPArgCheckRes;
 
 /*
+ * Check if client's argument exists and type is correct
+ */
+static void check_client_args_type(const char *client_arg_name,
+   QObject *client_arg, void *opaque)
+{
+QObject *obj;
+QString *arg_type;
+QMPArgCheckRes *res = opaque;
+
+if (res->result < 0) {
+/* report only the first error */
+return;
+}
+
+obj = qdict_get(res->qdict, client_arg_name);
+if (!obj) {
+/* client arg doesn't exist */
+res->result = -1;
+qerror_report(QERR_INVALID_PARAMETER, client_arg_name);
+return;
+}
+
+arg_type = qobject_to_qstring(obj);
+assert(arg_type != NULL);
+
+/* check if argument's type is correct */
+switch (qstring_get_str(arg_type)[0]) {
+case 'F':
+case 'B':
+case 's':
+if (qobject_type(client_arg) != QTYPE_QSTRING) {
+qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name,
+  "string");
+res->result = -1;
+}
+break;
+case 'i':
+case 'l':
+case 'M':
+if (qobject_type(client_arg) != QTYPE_QINT) {
+qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name, "int");
+res->result = -1;
+}
+break;
+case 'f':
+case 'T':
+if (qobject_type(client_arg) != QTYPE_QINT &&
+qobject_type(client_arg) != QTYPE_QFLOAT) {
+qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name,
+  "number");
+res->result = -1;
+}
+break;
+case 'b':
+case '-':
+if (qobject_type(client_arg) != QTYPE_QBOOL) {
+qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name, 
"bool");
+res->result = -1;
+}
+break;
+case 'O':
+/* Not checked here */
+break;
+default:
+abort();
+}
+}
+
+/*
  * Check if client passed all mandatory args
  */
 static void check_mandatory_args(const char *cmd_arg_name,
@@ -4344,6 +4413,9 @@ out:
  * Client argument checking rules:
  *
  * 1. Client must provide all mandatory arguments
+ * 2. Each argument provided by the client must be valid
+ * 3. Each argument provided by the client must have the type expected
+ *by the command
  */
 static int qmp_check_client_args(const mon_cmd_t *cmd, QDict *client_args)
 {
@@ -4355,7 +4427,10 @@ static int qmp_check_client_args(const mon_cmd_t *cmd, 
QDict *client_args)
 res.qdict = client_args;
 qdict_iter(cmd_args, check_mandatory_args, &res);
 
-/* TODO: Check client args type */
+if (!res.result && !res.skip) {
+res.qdict = cmd_args;
+qdict_iter(client_args, check_client_args_type, &res);
+}
 
 QDECREF(cmd_args);
 return res.result;
-- 
1.7.1.231.gd0b16




Re: [Qemu-devel] [PATCH 2/8] sparc64: fix missing address masking

2010-06-01 Thread Richard Henderson
On 06/01/2010 01:12 PM, Igor V. Kovalenko wrote:
> +if ((env->pstate & PS_AM) && is_translating_asi(asi)) {
> +addr &= 0xULL;
> +}

I suggest that these be written instead as

  if (is_translating_asi(asi)) {
addr = address_mask(addr);
  }

That should allow you to remove some of the ifdefs.


r~



[Qemu-devel] [PATCH 7/9] QError: Introduce QERR_QMP_BAD_INPUT_OBJECT_MEMBER

2010-06-01 Thread Luiz Capitulino
Signed-off-by: Luiz Capitulino 
---
 qerror.c |4 
 qerror.h |3 +++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/qerror.c b/qerror.c
index 44d0bf8..b26224e 100644
--- a/qerror.c
+++ b/qerror.c
@@ -177,6 +177,10 @@ static const QErrorStringTable qerror_table[] = {
 .desc  = "QMP input object member '%(member)' expects 
'%(expected)'",
 },
 {
+.error_fmt = QERR_QMP_INVALID_INPUT_OBJECT_MEMBER,
+.desc  = "QMP input object member '%(member)' is invalid",
+},
+{
 .error_fmt = QERR_SET_PASSWD_FAILED,
 .desc  = "Could not set password",
 },
diff --git a/qerror.h b/qerror.h
index 77ae574..eec9949 100644
--- a/qerror.h
+++ b/qerror.h
@@ -148,6 +148,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_QMP_BAD_INPUT_OBJECT_MEMBER \
 "{ 'class': 'QMPBadInputObjectMember', 'data': { 'member': %s, 'expected': 
%s } }"
 
+#define QERR_QMP_INVALID_INPUT_OBJECT_MEMBER \
+"{ 'class': 'QMPInvalidInputObjectMember', 'data': { 'member': %s } }"
+
 #define QERR_SET_PASSWD_FAILED \
 "{ 'class': 'SetPasswdFailed', 'data': {} }"
 
-- 
1.7.1.231.gd0b16




[Qemu-devel] [PATCH 5/9] QMP: Drop old client argument checker

2010-06-01 Thread Luiz Capitulino
Previous two commits added qmp_check_client_args(), which
fully replaces this code and is way better.

It's important to note that the new checker doesn't support
the '/' arg type. As we don't have any of those handlers
converted to QMP, this is just dead code.

Signed-off-by: Luiz Capitulino 
---
 monitor.c |  170 -
 1 files changed, 0 insertions(+), 170 deletions(-)

diff --git a/monitor.c b/monitor.c
index 14790e6..d2a510e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4082,177 +4082,12 @@ static int monitor_can_read(void *opaque)
 return (mon->suspend_cnt == 0) ? 1 : 0;
 }
 
-typedef struct CmdArgs {
-QString *name;
-int type;
-int flag;
-int optional;
-} CmdArgs;
-
-static int check_opt(const CmdArgs *cmd_args, const char *name, QDict *args)
-{
-if (!cmd_args->optional) {
-qerror_report(QERR_MISSING_PARAMETER, name);
-return -1;
-}
-
-return 0;
-}
-
-static int check_arg(const CmdArgs *cmd_args, QDict *args)
-{
-QObject *value;
-const char *name;
-
-name = qstring_get_str(cmd_args->name);
-
-if (!args) {
-return check_opt(cmd_args, name, args);
-}
-
-value = qdict_get(args, name);
-if (!value) {
-return check_opt(cmd_args, name, args);
-}
-
-switch (cmd_args->type) {
-case 'F':
-case 'B':
-case 's':
-if (qobject_type(value) != QTYPE_QSTRING) {
-qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "string");
-return -1;
-}
-break;
-case '/': {
-int i;
-const char *keys[] = { "count", "format", "size", NULL };
-
-for (i = 0; keys[i]; i++) {
-QObject *obj = qdict_get(args, keys[i]);
-if (!obj) {
-qerror_report(QERR_MISSING_PARAMETER, name);
-return -1;
-}
-if (qobject_type(obj) != QTYPE_QINT) {
-qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "int");
-return -1;
-}
-}
-break;
-}
-case 'i':
-case 'l':
-case 'M':
-if (qobject_type(value) != QTYPE_QINT) {
-qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "int");
-return -1;
-}
-break;
-case 'f':
-case 'T':
-if (qobject_type(value) != QTYPE_QINT && qobject_type(value) != 
QTYPE_QFLOAT) {
-qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "number");
-return -1;
-}
-break;
-case 'b':
-if (qobject_type(value) != QTYPE_QBOOL) {
-qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "bool");
-return -1;
-}
-break;
-case '-':
-if (qobject_type(value) != QTYPE_QINT &&
-qobject_type(value) != QTYPE_QBOOL) {
-qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "bool");
-return -1;
-}
-break;
-case 'O':
-default:
-/* impossible */
-abort();
-}
-
-return 0;
-}
-
-static void cmd_args_init(CmdArgs *cmd_args)
-{
-cmd_args->name = qstring_new();
-cmd_args->type = cmd_args->flag = cmd_args->optional = 0;
-}
-
 static int check_opts(QemuOptsList *opts_list, QDict *args)
 {
 assert(!opts_list->desc->name);
 return 0;
 }
 
-/*
- * This is not trivial, we have to parse Monitor command's argument
- * type syntax to be able to check the arguments provided by clients.
- *
- * In the near future we will be using an array for that and will be
- * able to drop all this parsing...
- */
-static int monitor_check_qmp_args(const mon_cmd_t *cmd, QDict *args)
-{
-int err;
-const char *p;
-CmdArgs cmd_args;
-QemuOptsList *opts_list;
-
-if (cmd->args_type == NULL) {
-return (qdict_size(args) == 0 ? 0 : -1);
-}
-
-err = 0;
-cmd_args_init(&cmd_args);
-opts_list = NULL;
-
-for (p = cmd->args_type;; p++) {
-if (*p == ':') {
-cmd_args.type = *++p;
-p++;
-if (cmd_args.type == '-') {
-cmd_args.flag = *p++;
-cmd_args.optional = 1;
-} else if (cmd_args.type == 'O') {
-opts_list = qemu_find_opts(qstring_get_str(cmd_args.name));
-assert(opts_list);
-} else if (*p == '?') {
-cmd_args.optional = 1;
-p++;
-}
-
-assert(*p == ',' || *p == '\0');
-if (opts_list) {
-err = check_opts(opts_list, args);
-opts_list = NULL;
-} else {
-err = check_arg(&cmd_args, args);
-QDECREF(cmd_args.name);
-cmd_args_init(&cmd_args

[Qemu-devel] [PATCH 9/9] QMP: Drop old input object checking code

2010-06-01 Thread Luiz Capitulino
Previous commit added qmp_check_input_obj(), it does this
checking for us.

Signed-off-by: Luiz Capitulino 
---
 monitor.c |6 --
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/monitor.c b/monitor.c
index 654b193..f849456 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4347,9 +4347,6 @@ static void handle_qmp_command(JSONMessageParser *parser, 
QList *tokens)
 if (!obj) {
 qerror_report(QERR_QMP_BAD_INPUT_OBJECT, "execute");
 goto err_input;
-} else if (qobject_type(obj) != QTYPE_QSTRING) {
-qerror_report(QERR_QMP_BAD_INPUT_OBJECT_MEMBER, "execute", "string");
-goto err_input;
 }
 
 cmd_name = qstring_get_str(qobject_to_qstring(obj));
@@ -4381,9 +4378,6 @@ static void handle_qmp_command(JSONMessageParser *parser, 
QList *tokens)
 obj = qdict_get(input, "arguments");
 if (!obj) {
 args = qdict_new();
-} else if (qobject_type(obj) != QTYPE_QDICT) {
-qerror_report(QERR_QMP_BAD_INPUT_OBJECT_MEMBER, "arguments", "object");
-goto err_input;
 } else {
 args = qobject_to_qdict(obj);
 QINCREF(args);
-- 
1.7.1.231.gd0b16




[Qemu-devel] [PATCH 6/9] QMP: check_opts(): Minor cleanup

2010-06-01 Thread Luiz Capitulino
We couldn't do it before, otherwise we would break the intention
of the previous checker, which was to ensure that opts_list wasn't
a NULL before checking it.

Debug code, pretty minor, still I decided to maintain its original
behavior.

Signed-off-by: Luiz Capitulino 
---
 monitor.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index d2a510e..1875731 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4084,6 +4084,7 @@ static int monitor_can_read(void *opaque)
 
 static int check_opts(QemuOptsList *opts_list, QDict *args)
 {
+assert(opts_list);
 assert(!opts_list->desc->name);
 return 0;
 }
@@ -4188,7 +4189,6 @@ static void check_mandatory_args(const char *cmd_arg_name,
 
 if (qstring_get_str(type)[0] == 'O') {
 QemuOptsList *opts_list = qemu_find_opts(cmd_arg_name);
-assert(opts_list);
 res->result = check_opts(opts_list, res->qdict);
 res->skip = 1;
 } else if (qstring_get_str(type)[0] != '-' &&
-- 
1.7.1.231.gd0b16




[Qemu-devel] [PATCH 2/9] Monitor: handle optional '-' arg as a bool

2010-06-01 Thread Luiz Capitulino
Historically, user monitor arguments beginning with '-' (eg. '-f')
were passed as integers down to handlers.

I've maintained this behavior in the new monitor because we didn't
have a boolean type at the very beginning of QMP. Today we have it
and this behavior is causing trouble to the code that checks QMP
client's arguments.

This commit fixes the problem by doing the following changes:

1. User Monitor

   Before: the optional arg was represented as a QInt, we'd pass 1
   down to handlers if the user specified the argument or
   0 otherwise

   This commit: the optional arg is represented as a QBool, we pass
true down to handlers if the user specified the
argument, otherwise _nothing_ is passed

2. QMP

   Before: the client was required to pass the arg as QBool, but we'd
   convert it to QInt internally. If the argument wasn't passed,
   we'd pass 0 down

   This commit: keep requiring a QBool, but doesn't do any conversion
and doesn't pass any default value

3. Convert existing handlers (do_eject()/do_migrate()) to the new way

   Before: Both handlers would expect QInt value, either 0 or 1

   This commit: Change the handlers to accept a QBool, they handle the
following cases:

   A) true is passed: the option is enabled
   B) false is passed: the option is disabled
   C) nothing is passed: option not specified, use
 default behavior

Signed-off-by: Luiz Capitulino 
---
 migration.c |   16 +++-
 monitor.c   |   19 ---
 2 files changed, 11 insertions(+), 24 deletions(-)

diff --git a/migration.c b/migration.c
index 706fe55..efecbdc 100644
--- a/migration.c
+++ b/migration.c
@@ -58,7 +58,9 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject 
**ret_data)
 {
 MigrationState *s = NULL;
 const char *p;
-int detach = qdict_get_int(qdict, "detach");
+int detach = qdict_get_try_bool(qdict, "detach", 0);
+int blk = qdict_get_try_bool(qdict, "blk", 0);
+int inc = qdict_get_try_bool(qdict, "inc", 0);
 const char *uri = qdict_get_str(qdict, "uri");
 
 if (current_migration &&
@@ -69,21 +71,17 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject 
**ret_data)
 
 if (strstart(uri, "tcp:", &p)) {
 s = tcp_start_outgoing_migration(mon, p, max_throttle, detach,
- (int)qdict_get_int(qdict, "blk"), 
- (int)qdict_get_int(qdict, "inc"));
+ blk, inc);
 #if !defined(WIN32)
 } else if (strstart(uri, "exec:", &p)) {
 s = exec_start_outgoing_migration(mon, p, max_throttle, detach,
-  (int)qdict_get_int(qdict, "blk"), 
-  (int)qdict_get_int(qdict, "inc"));
+  blk, inc);
 } else if (strstart(uri, "unix:", &p)) {
 s = unix_start_outgoing_migration(mon, p, max_throttle, detach,
- (int)qdict_get_int(qdict, "blk"), 
-  (int)qdict_get_int(qdict, "inc"));
+  blk, inc);
 } else if (strstart(uri, "fd:", &p)) {
 s = fd_start_outgoing_migration(mon, p, max_throttle, detach, 
-(int)qdict_get_int(qdict, "blk"), 
-(int)qdict_get_int(qdict, "inc"));
+blk, inc);
 #endif
 } else {
 monitor_printf(mon, "unknown migration protocol: %s\n", uri);
diff --git a/monitor.c b/monitor.c
index 15b53b9..bc3cc18 100644
--- a/monitor.c
+++ b/monitor.c
@@ -971,7 +971,7 @@ static int eject_device(Monitor *mon, BlockDriverState *bs, 
int force)
 static int do_eject(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
 BlockDriverState *bs;
-int force = qdict_get_int(qdict, "force");
+int force = qdict_get_try_bool(qdict, "force", 0);
 const char *filename = qdict_get_str(qdict, "device");
 
 bs = bdrv_find(filename);
@@ -3684,7 +3684,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
*mon,
 case '-':
 {
 const char *tmp = p;
-int has_option, skip_key = 0;
+int skip_key = 0;
 /* option */
 
 c = *typestr++;
@@ -3692,7 +3692,6 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
*mon,
 goto bad_type;
 while (qemu_isspace(*p))
 p++;
-has_option = 0;
 if (*p == '-') {
 p++;
 if(c != *p) {
@@ -3708,11 +3707,11 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
*mon,
 if(skip_key) {
  

[Qemu-devel] [PATCH 8/9] QMP: Introduce qmp_check_input_obj()

2010-06-01 Thread Luiz Capitulino
This is similar to qmp_check_client_args(), but checks if
the input object follows the specification (QMP/qmp-spec.txt
section 2.3).

As we're limited to three keys, the work here is quite simple:
we iterate over the input object, each time checking if the
given argument complies to the specification.

Signed-off-by: Luiz Capitulino 
---
 monitor.c |   45 +
 1 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index 1875731..654b193 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4271,6 +4271,45 @@ static int qmp_check_client_args(const mon_cmd_t *cmd, 
QDict *client_args)
 return res.result;
 }
 
+/*
+ * Input object checking rules
+ *
+ * 1. "execute" key must exist (not checked here)
+ * 2. "execute" key must be a string
+ * 3. "arguments" key must be a dict
+ * 4. "id" key can be anything (ie. json-value)
+ * 5. Any argument not listed above is invalid
+ */
+static void qmp_check_input_obj(const char *input_obj_arg_name,
+QObject *input_obj_arg, void *opaque)
+{
+int *err = opaque;
+
+if (*err < 0) {
+/* report only the first error */
+return;
+}
+
+if (!strcmp(input_obj_arg_name, "execute")) {
+if (qobject_type(input_obj_arg) != QTYPE_QSTRING) {
+qerror_report(QERR_QMP_BAD_INPUT_OBJECT_MEMBER, "execute",
+  "string");
+*err = -1;
+}
+} else if (!strcmp(input_obj_arg_name, "arguments")) {
+if (qobject_type(input_obj_arg) != QTYPE_QDICT) {
+qerror_report(QERR_QMP_BAD_INPUT_OBJECT_MEMBER, "arguments",
+  "object");
+*err = -1;
+}
+} else if (!strcmp(input_obj_arg_name, "id")) {
+/* nothing to do */
+} else {
+qerror_report(QERR_QMP_INVALID_INPUT_OBJECT_MEMBER, 
input_obj_arg_name);
+*err = -1;
+}
+}
+
 static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
 {
 int err;
@@ -4295,6 +4334,12 @@ static void handle_qmp_command(JSONMessageParser 
*parser, QList *tokens)
 
 input = qobject_to_qdict(obj);
 
+err = 0;
+qdict_iter(input, qmp_check_input_obj, &err);
+if (err < 0) {
+goto err_out;
+}
+
 mon->mc->id = qdict_get(input, "id");
 qobject_incref(mon->mc->id);
 
-- 
1.7.1.231.gd0b16




[Qemu-devel] [PATCH 3/9] QMP: First half of the new argument checking code

2010-06-01 Thread Luiz Capitulino
This commit introduces the first half of qmp_check_client_args(),
which is the new client argument checker.

It's introduced on top of the existing code, so that there are
no regressions during the transition.

It works this way: the command's args_type field (from
qemu-monitor.hx) is transformed into a qdict. Then we iterate
over it checking whether each mandatory argument has been
provided by the client.

All comunication between the iterator and its caller is done
via the new QMPArgCheckRes type.

Next commit adds the second half of this work: type checking
and invalid argument detection.

Signed-off-by: Luiz Capitulino 
---
 monitor.c |  107 +
 1 files changed, 107 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index bc3cc18..47a0da8 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4259,6 +4259,108 @@ static int invalid_qmp_mode(const Monitor *mon, const 
char *cmd_name)
 return (qmp_cmd_mode(mon) ? is_cap : !is_cap);
 }
 
+typedef struct QMPArgCheckRes {
+int result;
+int skip;
+QDict *qdict;
+} QMPArgCheckRes;
+
+/*
+ * Check if client passed all mandatory args
+ */
+static void check_mandatory_args(const char *cmd_arg_name,
+ QObject *obj, void *opaque)
+{
+QString *type;
+QMPArgCheckRes *res = opaque;
+
+if (res->result < 0) {
+/* report only the first error */
+return;
+}
+
+type = qobject_to_qstring(obj);
+assert(type != NULL);
+
+if (qstring_get_str(type)[0] == 'O') {
+QemuOptsList *opts_list = qemu_find_opts(cmd_arg_name);
+assert(opts_list);
+res->result = check_opts(opts_list, res->qdict);
+res->skip = 1;
+} else if (qstring_get_str(type)[0] != '-' &&
+   qstring_get_str(type)[1] != '?' &&
+   !qdict_haskey(res->qdict, cmd_arg_name)) {
+res->result = -1;
+qerror_report(QERR_MISSING_PARAMETER, cmd_arg_name);
+}
+}
+
+static QDict *qdict_from_args_type(const char *args_type)
+{
+int i;
+QDict *qdict;
+QString *key, *type, *cur_qs;
+
+assert(args_type != NULL);
+
+qdict = qdict_new();
+
+if (args_type == NULL || args_type[0] == '\0') {
+/* no args, empty qdict */
+goto out;
+}
+
+key = qstring_new();
+type = qstring_new();
+
+cur_qs = key;
+
+for (i = 0;; i++) {
+switch (args_type[i]) {
+case ',':
+case '\0':
+qdict_put(qdict, qstring_get_str(key), type);
+QDECREF(key);
+if (args_type[i] == '\0') {
+goto out;
+}
+type = qstring_new(); /* qdict has ref */
+cur_qs = key = qstring_new();
+break;
+case ':':
+cur_qs = type;
+break;
+default:
+qstring_append_chr(cur_qs, args_type[i]);
+break;
+}
+}
+
+out:
+return qdict;
+}
+
+/*
+ * Client argument checking rules:
+ *
+ * 1. Client must provide all mandatory arguments
+ */
+static int qmp_check_client_args(const mon_cmd_t *cmd, QDict *client_args)
+{
+QDict *cmd_args;
+QMPArgCheckRes res = { .result = 0, .skip = 0 };
+
+cmd_args = qdict_from_args_type(cmd->args_type);
+
+res.qdict = client_args;
+qdict_iter(cmd_args, check_mandatory_args, &res);
+
+/* TODO: Check client args type */
+
+QDECREF(cmd_args);
+return res.result;
+}
+
 static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
 {
 int err;
@@ -4334,6 +4436,11 @@ static void handle_qmp_command(JSONMessageParser 
*parser, QList *tokens)
 
 QDECREF(input);
 
+err = qmp_check_client_args(cmd, args);
+if (err < 0) {
+goto err_out;
+}
+
 err = monitor_check_qmp_args(cmd, args);
 if (err < 0) {
 goto err_out;
-- 
1.7.1.231.gd0b16




[Qemu-devel] [PATCH 0/9]: QMP: Replace client argument checker

2010-06-01 Thread Luiz Capitulino
Current QMP's client argument checker implementation is more complex than it
should be and has a flaw: it ignores unknown arguments.

This series solves both problems by introducing a new, simple and ultra-poweful
argument checker. This wasn't trivial to get right due to the number of errors
combinations, so review is very appreciated.




[Qemu-devel] [PATCH 1/9] QDict: Introduce qdict_get_try_bool()

2010-06-01 Thread Luiz Capitulino
Signed-off-by: Luiz Capitulino 
---
 qdict.c |   18 ++
 qdict.h |1 +
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/qdict.c b/qdict.c
index 175bc17..ca3c3b1 100644
--- a/qdict.c
+++ b/qdict.c
@@ -287,6 +287,24 @@ int64_t qdict_get_try_int(const QDict *qdict, const char 
*key,
 }
 
 /**
+ * qdict_get_try_bool(): Try to get a bool mapped by 'key'
+ *
+ * Return bool mapped by 'key', if it is not present in the
+ * dictionary or if the stored object is not of QBool type
+ * 'err_value' will be returned.
+ */
+int qdict_get_try_bool(const QDict *qdict, const char *key, int err_value)
+{
+QObject *obj;
+
+obj = qdict_get(qdict, key);
+if (!obj || qobject_type(obj) != QTYPE_QBOOL)
+return err_value;
+
+return qbool_get_int(qobject_to_qbool(obj));
+}
+
+/**
  * qdict_get_try_str(): Try to get a pointer to the stored string
  * mapped by 'key'
  *
diff --git a/qdict.h b/qdict.h
index 5e5902c..5cca816 100644
--- a/qdict.h
+++ b/qdict.h
@@ -57,6 +57,7 @@ QDict *qdict_get_qdict(const QDict *qdict, const char *key);
 const char *qdict_get_str(const QDict *qdict, const char *key);
 int64_t qdict_get_try_int(const QDict *qdict, const char *key,
   int64_t err_value);
+int qdict_get_try_bool(const QDict *qdict, const char *key, int err_value);
 const char *qdict_get_try_str(const QDict *qdict, const char *key);
 
 #endif /* QDICT_H */
-- 
1.7.1.231.gd0b16




[Qemu-devel] Re: [PATCH] x86: svm: Always clear event_inj on vmexit

2010-06-01 Thread Erik van der Kouwe

Hi,

> We currently only clear SVM_EVTINJ_VALID after successful interrupt
> delivery. This apparently does not match real hardware which clears the
> whole event_inj field on every vmexit, including unsuccessful interrupt
> delivery.

Thanks for the patch. It is a bit hard for me to test right now as I 
messed up my test setup, but I will do so ASAP and let you know.


However, I'm worried that this patch may introduce a new problem (I may 
be mistaken though). There is still this code to load the exit interrupt 
info:


stl_phys(env->vm_vmcb + offsetof(struct vmcb,
  control.exit_int_info_err),
  ldl_phys(env->vm_vmcb + offsetof(struct vmcb,
  control.event_inj_err)));

Now that event_inj is no longer loaded, won't this mean that 
exit_int_info and exit_int_info_err also won't be loaded?


With kind regards,
Erik

Jan Kiszka wrote:

We currently only clear SVM_EVTINJ_VALID after successful interrupt
delivery. This apparently does not match real hardware which clears the
whole event_inj field on every vmexit, including unsuccessful interrupt
delivery.

Reported-by: Erik van der Kouwe 
Signed-off-by: Jan Kiszka 
---

(before it gets lost)
Erik, please confirm that this works for you.

 target-i386/op_helper.c |8 +---
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
index dcbdfe7..caabdb4 100644
--- a/target-i386/op_helper.c
+++ b/target-i386/op_helper.c
@@ -1263,13 +1263,6 @@ void do_interrupt(int intno, int is_int, int error_code,
 #endif
 do_interrupt_real(intno, is_int, error_code, next_eip);
 }
-
-#if !defined(CONFIG_USER_ONLY)
-if (env->hflags & HF_SVMI_MASK) {
-   uint32_t event_inj = ldl_phys(env->vm_vmcb + offsetof(struct vmcb, 
control.event_inj));
-   stl_phys(env->vm_vmcb + offsetof(struct vmcb, control.event_inj), 
event_inj & ~SVM_EVTINJ_VALID);
-}
-#endif
 }
 
 /* This should come from sysemu.h - if we could include it here... */

@@ -5388,6 +5381,7 @@ void helper_vmexit(uint32_t exit_code, uint64_t 
exit_info_1)
  ldl_phys(env->vm_vmcb + offsetof(struct vmcb, 
control.event_inj)));
 stl_phys(env->vm_vmcb + offsetof(struct vmcb, control.exit_int_info_err),
  ldl_phys(env->vm_vmcb + offsetof(struct vmcb, 
control.event_inj_err)));
+stl_phys(env->vm_vmcb + offsetof(struct vmcb, control.event_inj), 0);
 
 env->hflags2 &= ~HF2_GIF_MASK;

 /* FIXME: Resets the current ASID register to zero (host ASID). */





[Qemu-devel] [PATCH] virtio-9p: Rearrange fileop structures

2010-06-01 Thread Venkateswararao Jujjuri (JV)
This patch rearranges the fileop structures by moving the structure definitions
from virtio-9p.c to virtio-9p.h file. No functional changes.

Signed-off-by: Venkateswararao Jujjuri 
---
 hw/virtio-9p.c |  185 ++--
 hw/virtio-9p.h |   92 
 2 files changed, 138 insertions(+), 139 deletions(-)

diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index e5d0112..038bb39 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -21,6 +21,52 @@
 int dotu = 1;
 int debug_9p_pdu;
 
+enum {
+Oread   = 0x00,
+Owrite  = 0x01,
+Ordwr   = 0x02,
+Oexec   = 0x03,
+Oexcl   = 0x04,
+Otrunc  = 0x10,
+Orexec  = 0x20,
+Orclose = 0x40,
+Oappend = 0x80,
+};
+
+static int omode_to_uflags(int8_t mode)
+{
+int ret = 0;
+
+switch (mode & 3) {
+case Oread:
+ret = O_RDONLY;
+break;
+case Ordwr:
+ret = O_RDWR;
+break;
+case Owrite:
+ret = O_WRONLY;
+break;
+case Oexec:
+ret = O_RDONLY;
+break;
+}
+
+if (mode & Otrunc) {
+ret |= O_TRUNC;
+}
+
+if (mode & Oappend) {
+ret |= O_APPEND;
+}
+
+if (mode & Oexcl) {
+ret |= O_EXCL;
+}
+
+return ret;
+}
+
 static int v9fs_do_lstat(V9fsState *s, V9fsString *path, struct stat *stbuf)
 {
 return s->ops->lstat(&s->ctx, path->data, stbuf);
@@ -995,14 +1041,6 @@ out:
 v9fs_string_free(&aname);
 }
 
-typedef struct V9fsStatState {
-V9fsPDU *pdu;
-size_t offset;
-V9fsStat v9stat;
-V9fsFidState *fidp;
-struct stat stbuf;
-} V9fsStatState;
-
 static void v9fs_stat_post_lstat(V9fsState *s, V9fsStatState *vs, int err)
 {
 if (err == -1) {
@@ -1053,19 +1091,6 @@ out:
 qemu_free(vs);
 }
 
-typedef struct V9fsWalkState {
-V9fsPDU *pdu;
-size_t offset;
-int16_t nwnames;
-int name_idx;
-V9fsQID *qids;
-V9fsFidState *fidp;
-V9fsFidState *newfidp;
-V9fsString path;
-V9fsString *wnames;
-struct stat stbuf;
-} V9fsWalkState;
-
 static void v9fs_walk_complete(V9fsState *s, V9fsWalkState *vs, int err)
 {
 complete_pdu(s, vs->pdu, err);
@@ -1229,62 +1254,6 @@ out:
 v9fs_walk_complete(s, vs, err);
 }
 
-typedef struct V9fsOpenState {
-V9fsPDU *pdu;
-size_t offset;
-int8_t mode;
-V9fsFidState *fidp;
-V9fsQID qid;
-struct stat stbuf;
-
-} V9fsOpenState;
-
-enum {
-Oread   = 0x00,
-Owrite  = 0x01,
-Ordwr   = 0x02,
-Oexec   = 0x03,
-Oexcl   = 0x04,
-Otrunc  = 0x10,
-Orexec  = 0x20,
-Orclose = 0x40,
-Oappend = 0x80,
-};
-
-static int omode_to_uflags(int8_t mode)
-{
-int ret = 0;
-
-switch (mode & 3) {
-case Oread:
-ret = O_RDONLY;
-break;
-case Ordwr:
-ret = O_RDWR;
-break;
-case Owrite:
-ret = O_WRONLY;
-break;
-case Oexec:
-ret = O_RDONLY;
-break;
-}
-
-if (mode & Otrunc) {
-ret |= O_TRUNC;
-}
-
-if (mode & Oappend) {
-ret |= O_APPEND;
-}
-
-if (mode & Oexcl) {
-ret |= O_EXCL;
-}
-
-return ret;
-}
-
 static void v9fs_open_post_opendir(V9fsState *s, V9fsOpenState *vs, int err)
 {
 if (vs->fidp->dir == NULL) {
@@ -1387,25 +1356,6 @@ out:
 complete_pdu(s, pdu, err);
 }
 
-typedef struct V9fsReadState {
-V9fsPDU *pdu;
-size_t offset;
-int32_t count;
-int32_t total;
-int64_t off;
-V9fsFidState *fidp;
-struct iovec iov[128]; /* FIXME: bad, bad, bad */
-struct iovec *sg;
-off_t dir_pos;
-struct dirent *dent;
-struct stat stbuf;
-V9fsString name;
-V9fsStat v9stat;
-int32_t len;
-int32_t cnt;
-int32_t max_count;
-} V9fsReadState;
-
 static void v9fs_read_post_readdir(V9fsState *, V9fsReadState *, ssize_t);
 
 static void v9fs_read_post_seekdir(V9fsState *s, V9fsReadState *vs, ssize_t 
err)
@@ -1593,19 +1543,6 @@ out:
 qemu_free(vs);
 }
 
-typedef struct V9fsWriteState {
-V9fsPDU *pdu;
-size_t offset;
-int32_t len;
-int32_t count;
-int32_t total;
-int64_t off;
-V9fsFidState *fidp;
-struct iovec iov[128]; /* FIXME: bad, bad, bad */
-struct iovec *sg;
-int cnt;
-} V9fsWriteState;
-
 static void v9fs_write_post_writev(V9fsState *s, V9fsWriteState *vs,
ssize_t err)
 {
@@ -1702,19 +1639,6 @@ out:
 qemu_free(vs);
 }
 
-typedef struct V9fsCreateState {
-V9fsPDU *pdu;
-size_t offset;
-V9fsFidState *fidp;
-V9fsQID qid;
-int32_t perm;
-int8_t mode;
-struct stat stbuf;
-V9fsString name;
-V9fsString extension;
-V9fsString fullname;
-} V9fsCreateState;
-
 static void v9fs_post_create(V9fsState *s, V9fsCreateState *vs, int err)
 {
 if (err == 0) {
@@ -1934,12 +1858,6 @@ static void v9fs_flush(V9fsState *s, V9fsPDU *pdu)
 complete_pdu(s, pdu, 7);
 }
 
-typedef struct V9fsRemoveState {
-V9fsPDU *pdu;
-size_t offset;

[Qemu-devel] Re: [SeaBIOS] SMBIOS strings

2010-06-01 Thread Sebastian Herbszt

Jes Sorensen wrote:

Handle 0x0401, DMI type 4, 32 bytes
Processor Information
-   Socket Designation: CPU 1
+   Socket Designation: CPU01


smbios.c got
snprintf((char*)start, 6, "CPU%2x", cpu_number);

It should print "CPU 1" instead of "CPU01" because the
padding should be done with spaces not zeros. Maybe
bvprintf() doesn't handle it correctly?

Sebastian




[Qemu-devel] [PATCH] ahci: drop ATA_CMD constants

2010-06-01 Thread Sebastian Herbszt
Drop ATA_CMD constants and use constants from ide/internal.h.

Signed-off-by: Sebastian Herbszt 

diff --git a/hw/ahci.c b/hw/ahci.c
index 9987459..b4eafdf 100644
--- a/hw/ahci.c
+++ b/hw/ahci.c
@@ -153,33 +153,6 @@ do { fprintf(stderr,"ahci: " fmt , ## __VA_ARGS__); } 
while (0)
 #define STATE_RESET 1
 
 /*
- * ATA Commands (only mandatory commands listed here)
- */
-#define ATA_CMD_READ   0x20/* Read Sectors (with retries)  */
-#define ATA_CMD_READN  0x21/* Read Sectors ( no  retries)  */
-#define ATA_CMD_WRITE  0x30/* Write Sectores (with retries)*/
-#define ATA_CMD_WRITEN 0x31/* Write Sectors  ( no  retries)*/
-#define ATA_CMD_VRFY   0x40/* Read Verify  (with retries)  */
-#define ATA_CMD_VRFYN  0x41/* Read verify  ( no  retries)  */
-#define ATA_CMD_SEEK   0x70/* Seek */
-#define ATA_CMD_DIAG   0x90/* Execute Device Diagnostic*/
-#define ATA_CMD_INIT   0x91/* Initialize Device Parameters */
-#define ATA_CMD_RD_MULT0xC4/* Read Multiple*/
-#define ATA_CMD_WR_MULT0xC5/* Write Multiple   */
-#define ATA_CMD_SETMULT0xC6/* Set Multiple Mode*/
-#define ATA_CMD_RD_DMA 0xC8/* Read DMA (with retries)  */
-#define ATA_CMD_RD_DMAN0xC9/* Read DMS ( no  retries)  */
-#define ATA_CMD_WR_DMA 0xCA/* Write DMA (with retries) */
-#define ATA_CMD_WR_DMAN0xCB/* Write DMA ( no  retires) */
-#define ATA_CMD_IDENT  0xEC/* Identify Device  */
-#define ATA_CMD_SETF   0xEF/* Set Features */
-#define ATA_CMD_CHK_PWR0xE5/* Check Power Mode */
-
-#define ATA_CMD_READ_EXT 0x24  /* Read Sectors (with retries)  with 48bit 
addressing */
-#define ATA_CMD_WRITE_EXT  0x34/* Write Sectores (with retries) with 
48bit addressing */
-#define ATA_CMD_VRFY_EXT   0x42/* Read Verify  (with retries)  with 
48bit addressing */
-
-/*
  * ATAPI Commands
  */
 #define ATAPI_CMD_IDENT 0xA1 /* Identify AT Atachment Packed Interface Device 
*/
@@ -1067,7 +1040,7 @@ static void handle_cmd(AHCIState *s,int port,int slot)
pr->irq_stat |= (1<<2);
break;

-   case ATA_CMD_IDENT:
+   case WIN_IDENTIFY:
ide_identify(ide_state);
write_to_sglist(ide_state->identify_data, 
sizeof(ide_state->identify_data),s->prdt_buf,prdt_num);
pr->irq_stat |= (1<<2);
@@ -1091,7 +1064,7 @@ static void handle_cmd(AHCIState *s,int port,int slot)
case WIN_SETFEATURES:
pr->irq_stat |= (1<<2);
break;
-   case ATA_CMD_RD_DMA:
+   case WIN_READDMA:

sector_num=(((int64_t)fis[10])<<40)|(((int64_t)fis[9])<<32)|(fis[8]<<24)|(fis[6]<<16)|(fis[5]<<8)|fis[4];
nb_sectors=(fis[13]<<8)|fis[12];
if(!nb_sectors)nb_sectors=256;
@@ -1103,7 +1076,7 @@ static void handle_cmd(AHCIState *s,int port,int slot)
}
pr->irq_stat |= (1<<2);
break;
-   case ATA_CMD_WR_DMA:
+   case WIN_WRITEDMA:

sector_num=(((int64_t)fis[10])<<40)|(((int64_t)fis[9])<<32)|(fis[8]<<24)|(fis[6]<<16)|(fis[5]<<8)|fis[4];
nb_sectors=(fis[13]<<8)|fis[12];
if(!nb_sectors)nb_sectors=256;




[Qemu-devel] [PATCH] [virtio-9p] Flush the debug message out to the log file.

2010-06-01 Thread Venkateswararao Jujjuri (JV)
This patch fluesh the debug messages to the log file  at the end of each
debug message.

Signed-off-by: Venkateswararao Jujjuri 
---
 hw/virtio-9p-debug.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-9p-debug.c b/hw/virtio-9p-debug.c
index 2fb2673..5bfa689 100644
--- a/hw/virtio-9p-debug.c
+++ b/hw/virtio-9p-debug.c
@@ -481,4 +481,6 @@ void pprint_pdu(V9fsPDU *pdu)
 }
 
 fprintf(llogfile, ")\n");
+/* Flush the log message out */
+fseek(llogfile, 0L, SEEK_CUR);
 }
-- 
1.6.5.2




[Qemu-devel] [PATCH 7/8] sparc64: fix udiv and sdiv insns

2010-06-01 Thread Igor V. Kovalenko
From: Igor V. Kovalenko 

- truncate second operand to 32bit

Signed-off-by: Igor V. Kovalenko 
---
 target-sparc/op_helper.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index 83067ae..4c5155f 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -3307,7 +3307,7 @@ target_ulong helper_udiv(target_ulong a, target_ulong b)
 uint32_t x1;
 
 x0 = (a & 0x) | ((int64_t) (env->y) << 32);
-x1 = b;
+x1 = (b & 0x);
 
 if (x1 == 0) {
 raise_exception(TT_DIV_ZERO);
@@ -3329,7 +3329,7 @@ target_ulong helper_sdiv(target_ulong a, target_ulong b)
 int32_t x1;
 
 x0 = (a & 0x) | ((int64_t) (env->y) << 32);
-x1 = b;
+x1 = (b & 0x);
 
 if (x1 == 0) {
 raise_exception(TT_DIV_ZERO);




[Qemu-devel] [PATCH 5/8] sparc64: use symbolic name for MMU index

2010-06-01 Thread Igor V. Kovalenko
From: Igor V. Kovalenko 

Signed-off-by: Igor V. Kovalenko 
---
 target-sparc/op_helper.c |   28 
 1 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index f5e153d..b9af52b 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -3322,18 +3322,19 @@ void helper_stdf(target_ulong addr, int mem_idx)
 helper_check_align(addr, 7);
 #if !defined(CONFIG_USER_ONLY)
 switch (mem_idx) {
-case 0:
+case MMU_USER_IDX:
 stfq_user(addr, DT0);
 break;
-case 1:
+case MMU_KERNEL_IDX:
 stfq_kernel(addr, DT0);
 break;
 #ifdef TARGET_SPARC64
-case 2:
+case MMU_HYPV_IDX:
 stfq_hypv(addr, DT0);
 break;
 #endif
 default:
+printf("helper_stdf: need to check MMU idx %d\n", mem_idx);
 break;
 }
 #else
@@ -3346,18 +3347,19 @@ void helper_lddf(target_ulong addr, int mem_idx)
 helper_check_align(addr, 7);
 #if !defined(CONFIG_USER_ONLY)
 switch (mem_idx) {
-case 0:
+case MMU_USER_IDX:
 DT0 = ldfq_user(addr);
 break;
-case 1:
+case MMU_KERNEL_IDX:
 DT0 = ldfq_kernel(addr);
 break;
 #ifdef TARGET_SPARC64
-case 2:
+case MMU_HYPV_IDX:
 DT0 = ldfq_hypv(addr);
 break;
 #endif
 default:
+printf("helper_stdf: need to check MMU idx %d\n", mem_idx);
 break;
 }
 #else
@@ -3373,24 +3375,25 @@ void helper_ldqf(target_ulong addr, int mem_idx)
 helper_check_align(addr, 7);
 #if !defined(CONFIG_USER_ONLY)
 switch (mem_idx) {
-case 0:
+case MMU_USER_IDX:
 u.ll.upper = ldq_user(addr);
 u.ll.lower = ldq_user(addr + 8);
 QT0 = u.q;
 break;
-case 1:
+case MMU_KERNEL_IDX:
 u.ll.upper = ldq_kernel(addr);
 u.ll.lower = ldq_kernel(addr + 8);
 QT0 = u.q;
 break;
 #ifdef TARGET_SPARC64
-case 2:
+case MMU_HYPV_IDX:
 u.ll.upper = ldq_hypv(addr);
 u.ll.lower = ldq_hypv(addr + 8);
 QT0 = u.q;
 break;
 #endif
 default:
+printf("helper_stdf: need to check MMU idx %d\n", mem_idx);
 break;
 }
 #else
@@ -3408,24 +3411,25 @@ void helper_stqf(target_ulong addr, int mem_idx)
 helper_check_align(addr, 7);
 #if !defined(CONFIG_USER_ONLY)
 switch (mem_idx) {
-case 0:
+case MMU_USER_IDX:
 u.q = QT0;
 stq_user(addr, u.ll.upper);
 stq_user(addr + 8, u.ll.lower);
 break;
-case 1:
+case MMU_KERNEL_IDX:
 u.q = QT0;
 stq_kernel(addr, u.ll.upper);
 stq_kernel(addr + 8, u.ll.lower);
 break;
 #ifdef TARGET_SPARC64
-case 2:
+case MMU_HYPV_IDX:
 u.q = QT0;
 stq_hypv(addr, u.ll.upper);
 stq_hypv(addr + 8, u.ll.lower);
 break;
 #endif
 default:
+printf("helper_stdf: need to check MMU idx %d\n", mem_idx);
 break;
 }
 #else




[Qemu-devel] Re: [PATCH] sparc32 esp fix spurious interrupts in chip reset

2010-06-01 Thread Artyom Tarasenko
2010/6/1 Blue Swirl :
> On Tue, Jun 1, 2010 at 7:56 PM, Artyom Tarasenko
>  wrote:
>> 2010/6/1 Blue Swirl :
>>> On Sun, May 30, 2010 at 10:35 PM, Artyom Tarasenko
>>>  wrote:
 lower interrupt during chip reset. Otherwise the ESP_RSTAT register
 may get out of sync with the IRQ line status. This effect became
 visible after commit 65899fe3
>>>
>>> Hard reset handlers should not touch qemu_irqs, because on cold start,
>>> the receiving end may be unprepared to handle the signal.
>>
>> Wouldn't the real hardware lower irq on the hardware reset?
>
> Yes, but since qemu_irqs have no state, and on a cold start or system
> reset all other devices are guaranteed to be reset, the callback would
> be useless.
>
>> And if it would not, would it still clear the corresponding bit in
>> the ESP_RSTAT register?
>
> All registers are set to zero in the lines below.
>
>>
>>> See
>>> 0d0a7e69e853639b123798877e019c3c7ee6634a,
>>> bc26e55a6615dc594be425d293db40d5cdcdb84b and
>>> 42f1ced228c9b616cfa2b69846025271618e4ef5.
>>>
>>> For ESP there are two other sources of reset: signal from DMA and chip
>>> reset command. On those cases, lowering IRQ makes sense.
>>>
>>> So the correct fix is to refactor the reset handling a bit. Does this
>>> patch also fix your test case?
>>
>> It does, but
>>
>> +static void esp_soft_reset(DeviceState *d)
>> +{
>> +    ESPState *s = container_of(d, ESPState, busdev.qdev);
>> +
>> +    qemu_irq_lower(s->irq);
>>
>> Shouldn't it be esp_lower_irq(s)? What's going to happen to the
>> DMA_INTR bit if dma was the source of the irq?
>
> Again, the registers are zeroed in esp_hard_reset().

How does it zero the _DMA_ registers? And sparc32_dma does share the
IRQ line with ESP, doesn't it?

>
> Thanks for testing.
>



-- 
Regards,
Artyom Tarasenko

solaris/sparc under qemu blog: http://tyom.blogspot.com/



[Qemu-devel] [PATCH 6/8] sparc64: improve ldf and stf insns

2010-06-01 Thread Igor V. Kovalenko
From: Igor V. Kovalenko 

- implemented block load/store primary/secondary with user privilege

Signed-off-by: Igor V. Kovalenko 
---
 target-sparc/op_helper.c |   28 
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index b9af52b..83067ae 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -3161,6 +3161,20 @@ void helper_ldf_asi(target_ulong addr, int asi, int 
size, int rd)
 }
 
 return;
+case 0x70: // Block load primary, user privilege
+case 0x71: // Block load secondary, user privilege
+if (rd & 7) {
+raise_exception(TT_ILL_INSN);
+return;
+}
+helper_check_align(addr, 0x3f);
+for (i = 0; i < 16; i++) {
+*(uint32_t *)&env->fpr[rd++] = helper_ld_asi(addr, asi & 0x1f, 4,
+ 0);
+addr += 4;
+}
+
+return;
 default:
 break;
 }
@@ -3211,6 +3225,20 @@ void helper_stf_asi(target_ulong addr, int asi, int 
size, int rd)
 }
 
 return;
+case 0x70: // Block store primary, user privilege
+case 0x71: // Block store secondary, user privilege
+if (rd & 7) {
+raise_exception(TT_ILL_INSN);
+return;
+}
+helper_check_align(addr, 0x3f);
+for (i = 0; i < 16; i++) {
+val = *(uint32_t *)&env->fpr[rd++];
+helper_st_asi(addr, val, asi & 0x1f, 4);
+addr += 4;
+}
+
+return;
 default:
 break;
 }




[Qemu-devel] [PATCH 8/8] sparc64: fix umul and smul insns

2010-06-01 Thread Igor V. Kovalenko
From: Igor V. Kovalenko 

- truncate and sign or zero extend operands before multiplication
- factor out common code to gen_op_multiply() with parameter to sign/zero extend
- call gen_op_multiply from gen_op_umul and gen_op_smul

Signed-off-by: Igor V. Kovalenko 
---
 target-sparc/translate.c |   55 --
 1 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 0bc1a82..23f9519 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -662,50 +662,53 @@ static inline void gen_op_mulscc(TCGv dst, TCGv src1, 
TCGv src2)
 tcg_gen_mov_tl(dst, cpu_cc_dst);
 }
 
-static inline void gen_op_umul(TCGv dst, TCGv src1, TCGv src2)
+static inline void gen_op_multiply(TCGv dst, TCGv src1, TCGv src2, int 
sign_ext)
 {
+TCGv_i32 r_src1, r_src2;
 TCGv_i64 r_temp, r_temp2;
 
+r_src1 = tcg_temp_new_i32();
+r_src2 = tcg_temp_new_i32();
+
+tcg_gen_trunc_tl_i32(r_src1, src1);
+tcg_gen_trunc_tl_i32(r_src2, src2);
+
 r_temp = tcg_temp_new_i64();
 r_temp2 = tcg_temp_new_i64();
 
-tcg_gen_extu_tl_i64(r_temp, src2);
-tcg_gen_extu_tl_i64(r_temp2, src1);
+if (sign_ext) {
+tcg_gen_ext_i32_i64(r_temp, r_src2);
+tcg_gen_ext_i32_i64(r_temp2, r_src1);
+} else {
+tcg_gen_extu_i32_i64(r_temp, r_src2);
+tcg_gen_extu_i32_i64(r_temp2, r_src1);
+}
+
 tcg_gen_mul_i64(r_temp2, r_temp, r_temp2);
 
 tcg_gen_shri_i64(r_temp, r_temp2, 32);
 tcg_gen_trunc_i64_tl(cpu_tmp0, r_temp);
 tcg_temp_free_i64(r_temp);
 tcg_gen_andi_tl(cpu_y, cpu_tmp0, 0x);
-#ifdef TARGET_SPARC64
-tcg_gen_mov_i64(dst, r_temp2);
-#else
+
 tcg_gen_trunc_i64_tl(dst, r_temp2);
-#endif
+
 tcg_temp_free_i64(r_temp2);
+
+tcg_temp_free_i32(r_src1);
+tcg_temp_free_i32(r_src2);
 }
 
-static inline void gen_op_smul(TCGv dst, TCGv src1, TCGv src2)
+static inline void gen_op_umul(TCGv dst, TCGv src1, TCGv src2)
 {
-TCGv_i64 r_temp, r_temp2;
-
-r_temp = tcg_temp_new_i64();
-r_temp2 = tcg_temp_new_i64();
-
-tcg_gen_ext_tl_i64(r_temp, src2);
-tcg_gen_ext_tl_i64(r_temp2, src1);
-tcg_gen_mul_i64(r_temp2, r_temp, r_temp2);
+/* zero-extend truncated operands before multiplication */
+gen_op_multiply(dst, src1, src2, 0);
+}
 
-tcg_gen_shri_i64(r_temp, r_temp2, 32);
-tcg_gen_trunc_i64_tl(cpu_tmp0, r_temp);
-tcg_temp_free_i64(r_temp);
-tcg_gen_andi_tl(cpu_y, cpu_tmp0, 0x);
-#ifdef TARGET_SPARC64
-tcg_gen_mov_i64(dst, r_temp2);
-#else
-tcg_gen_trunc_i64_tl(dst, r_temp2);
-#endif
-tcg_temp_free_i64(r_temp2);
+static inline void gen_op_smul(TCGv dst, TCGv src1, TCGv src2)
+{
+/* sign-extend truncated operands before multiplication */
+gen_op_multiply(dst, src1, src2, 1);
 }
 
 #ifdef TARGET_SPARC64




[Qemu-devel] [PATCH 3/8] sparc64: fix 32bit load sign extension

2010-06-01 Thread Igor V. Kovalenko
From: Igor V. Kovalenko 

- change return type of ldl_* to uint32_t to prevent unwanted sign extension
  visible in sparc64 load alternate address space methods
- note this change makes ldl_* softmmu implementations match ldl_phys one
Signed-off-by: Igor V. Kovalenko 
---
 softmmu_header.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/softmmu_header.h b/softmmu_header.h
index 6a36e01..2f95c33 100644
--- a/softmmu_header.h
+++ b/softmmu_header.h
@@ -60,7 +60,7 @@
 #if DATA_SIZE == 8
 #define RES_TYPE uint64_t
 #else
-#define RES_TYPE int
+#define RES_TYPE uint32_t
 #endif
 
 #if ACCESS_TYPE == (NB_MMU_MODES + 1)




[Qemu-devel] [PATCH 2/8] sparc64: fix missing address masking

2010-06-01 Thread Igor V. Kovalenko
From: Igor V. Kovalenko 

- address masking for ldqf and stqf insns
- address masking for lddf and stdf insns
- address masking for translating ASI (Ultrasparc IIi)

Signed-off-by: Igor V. Kovalenko 
---
 target-sparc/op_helper.c |   47 ++
 target-sparc/translate.c |4 
 2 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index ef3504f..f5e153d 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -2315,6 +2315,25 @@ void helper_st_asi(target_ulong addr, target_ulong val, 
int asi, int size)
 
 #else /* CONFIG_USER_ONLY */
 
+/* Ultrasparc IIi translating asi
+   - note this list is defined by cpu implementation
+ */
+static inline int is_translating_asi(int asi)
+{
+switch (asi) {
+case 0x04 ... 0x11:
+case 0x18 ... 0x19:
+case 0x24 ... 0x2C:
+case 0x70 ... 0x73:
+case 0x78 ... 0x79:
+case 0x80 ... 0xFF:
+return 1;
+
+default:
+return 0;
+}
+}
+
 uint64_t helper_ld_asi(target_ulong addr, int asi, int size, int sign)
 {
 uint64_t ret = 0;
@@ -2330,7 +2349,12 @@ uint64_t helper_ld_asi(target_ulong addr, int asi, int 
size, int sign)
 && !(env->hpstate & HS_PRIV)))
 raise_exception(TT_PRIV_ACT);
 
+if ((env->pstate & PS_AM) && is_translating_asi(asi)) {
+addr &= 0xULL;
+}
+
 helper_check_align(addr, size - 1);
+
 switch (asi) {
 case 0x82: // Primary no-fault
 case 0x8a: // Primary no-fault LE
@@ -2681,7 +2705,12 @@ void helper_st_asi(target_ulong addr, target_ulong val, 
int asi, int size)
 && !(env->hpstate & HS_PRIV)))
 raise_exception(TT_PRIV_ACT);
 
+if ((env->pstate & PS_AM) && is_translating_asi(asi)) {
+addr &= 0xULL;
+}
+
 helper_check_align(addr, size - 1);
+
 /* Convert to little endian */
 switch (asi) {
 case 0x0c: // Nucleus Little Endian (LE)
@@ -3056,6 +3085,12 @@ void helper_ldda_asi(target_ulong addr, int asi, int rd)
 && !(env->hpstate & HS_PRIV)))
 raise_exception(TT_PRIV_ACT);
 
+#if defined (CONFIG_SPARC64)
+if ((env->pstate & PS_AM) && is_translating_asi(asi)) {
+addr &= 0xULL;
+}
+#endif
+
 switch (asi) {
 #if !defined(CONFIG_USER_ONLY)
 case 0x24: // Nucleus quad LDD 128 bit atomic
@@ -3102,6 +3137,12 @@ void helper_ldf_asi(target_ulong addr, int asi, int 
size, int rd)
 unsigned int i;
 target_ulong val;
 
+#if defined (CONFIG_SPARC64)
+if ((env->pstate & PS_AM) && is_translating_asi(asi)) {
+addr &= 0xULL;
+}
+#endif
+
 helper_check_align(addr, 3);
 switch (asi) {
 case 0xf0: // Block load primary
@@ -3144,6 +3185,12 @@ void helper_stf_asi(target_ulong addr, int asi, int 
size, int rd)
 unsigned int i;
 target_ulong val = 0;
 
+#if defined (CONFIG_SPARC64)
+if ((env->pstate & PS_AM) && is_translating_asi(asi)) {
+addr &= 0xULL;
+}
+#endif
+
 helper_check_align(addr, 3);
 switch (asi) {
 case 0xe0: // UA2007 Block commit store primary (cache flush)
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 72ca0b4..eff64d4 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -4490,6 +4490,7 @@ static void disas_sparc_insn(DisasContext * dc)
 
 CHECK_FPU_FEATURE(dc, FLOAT128);
 r_const = tcg_const_i32(dc->mem_idx);
+gen_address_mask(dc, cpu_addr);
 gen_helper_ldqf(cpu_addr, r_const);
 tcg_temp_free_i32(r_const);
 gen_op_store_QT0_fpr(QFPREG(rd));
@@ -4500,6 +4501,7 @@ static void disas_sparc_insn(DisasContext * dc)
 TCGv_i32 r_const;
 
 r_const = tcg_const_i32(dc->mem_idx);
+gen_address_mask(dc, cpu_addr);
 gen_helper_lddf(cpu_addr, r_const);
 tcg_temp_free_i32(r_const);
 gen_op_store_DT0_fpr(DFPREG(rd));
@@ -4635,6 +4637,7 @@ static void disas_sparc_insn(DisasContext * dc)
 CHECK_FPU_FEATURE(dc, FLOAT128);
 gen_op_load_fpr_QT0(QFPREG(rd));
 r_const = tcg_const_i32(dc->mem_idx);
+gen_address_mask(dc, cpu_addr);
 gen_helper_stqf(cpu_addr, r_const);
 tcg_temp_free_i32(r_const);
 }
@@ -4657,6 +4660,7 @@ static void disas_sparc_insn(DisasContext * dc)
 
 gen_op_load_fpr_DT0(DFPREG(rd));
 r_const = tcg_const_i32(dc->mem_idx);
+gen_address_mask(dc, cpu_addr);
 gen_helper_stdf(cpu_addr, r_const);
 tcg_temp_free_i32(r_const);
 }




[Qemu-devel] [PATCH 4/8] sparc64: fix ldxfsr insn

2010-06-01 Thread Igor V. Kovalenko
From: Igor V. Kovalenko 

- rearrange code to break from switch when appropriate
- allow deprecated ldfsr insn

Signed-off-by: Igor V. Kovalenko 
---
 target-sparc/translate.c |6 +-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index eff64d4..0bc1a82 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -4476,7 +4476,11 @@ static void disas_sparc_insn(DisasContext * dc)
 if (rd == 1) {
 tcg_gen_qemu_ld64(cpu_tmp64, cpu_addr, dc->mem_idx);
 gen_helper_ldxfsr(cpu_tmp64);
-} else
+} else {
+tcg_gen_qemu_ld32u(cpu_tmp0, cpu_addr, dc->mem_idx);
+tcg_gen_trunc_tl_i32(cpu_tmp32, cpu_tmp0);
+gen_helper_ldfsr(cpu_tmp32);
+}
 #else
 {
 tcg_gen_qemu_ld32u(cpu_tmp32, cpu_addr, dc->mem_idx);




[Qemu-devel] [PATCH 0/8] sparc64 fixes

2010-06-01 Thread Igor V. Kovalenko
assorted changes, linux kernel can now work with 32bit init task

---

Igor V. Kovalenko (8):
  sparc64: fix tag access register on mmu traps
  sparc64: fix missing address masking
  sparc64: fix 32bit load sign extension
  sparc64: fix ldxfsr insn
  sparc64: use symbolic name for MMU index
  sparc64: improve ldf and stf insns
  sparc64: fix udiv and sdiv insns
  sparc64: fix umul and smul insns


 softmmu_header.h |2 -
 target-sparc/helper.c|5 ++
 target-sparc/op_helper.c |  107 --
 target-sparc/translate.c |   65 
 4 files changed, 137 insertions(+), 42 deletions(-)

-- 



[Qemu-devel] [PATCH 1/8] sparc64: fix tag access register on mmu traps

2010-06-01 Thread Igor V. Kovalenko
From: Igor V. Kovalenko 

- set mmu tag access register on FAULT and PROT traps as well

Signed-off-by: Igor V. Kovalenko 
---
 target-sparc/helper.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/target-sparc/helper.c b/target-sparc/helper.c
index 96a22f3..aa1fd63 100644
--- a/target-sparc/helper.c
+++ b/target-sparc/helper.c
@@ -495,6 +495,9 @@ static int get_physical_address_data(CPUState *env,
 env->dmmu.sfsr |= (fault_type << 7);
 
 env->dmmu.sfar = address; /* Fault address register */
+
+env->dmmu.tag_access = (address & ~0x1fffULL) | context;
+
 return 1;
 }
 }
@@ -544,6 +547,8 @@ static int get_physical_address_code(CPUState *env,
 env->immu.sfsr |= (is_user << 3) | 1;
 env->exception_index = TT_TFAULT;
 
+env->immu.tag_access = (address & ~0x1fffULL) | context;
+
 DPRINTF_MMU("TFAULT at %" PRIx64 " context %" PRIx64 "\n",
 address, context);
 




[Qemu-devel] Re: [PATCH] sparc32 esp fix spurious interrupts in chip reset

2010-06-01 Thread Blue Swirl
On Tue, Jun 1, 2010 at 7:56 PM, Artyom Tarasenko
 wrote:
> 2010/6/1 Blue Swirl :
>> On Sun, May 30, 2010 at 10:35 PM, Artyom Tarasenko
>>  wrote:
>>> lower interrupt during chip reset. Otherwise the ESP_RSTAT register
>>> may get out of sync with the IRQ line status. This effect became
>>> visible after commit 65899fe3
>>
>> Hard reset handlers should not touch qemu_irqs, because on cold start,
>> the receiving end may be unprepared to handle the signal.
>
> Wouldn't the real hardware lower irq on the hardware reset?

Yes, but since qemu_irqs have no state, and on a cold start or system
reset all other devices are guaranteed to be reset, the callback would
be useless.

> And if it would not, would it still clear the corresponding bit in
> the ESP_RSTAT register?

All registers are set to zero in the lines below.

>
>> See
>> 0d0a7e69e853639b123798877e019c3c7ee6634a,
>> bc26e55a6615dc594be425d293db40d5cdcdb84b and
>> 42f1ced228c9b616cfa2b69846025271618e4ef5.
>>
>> For ESP there are two other sources of reset: signal from DMA and chip
>> reset command. On those cases, lowering IRQ makes sense.
>>
>> So the correct fix is to refactor the reset handling a bit. Does this
>> patch also fix your test case?
>
> It does, but
>
> +static void esp_soft_reset(DeviceState *d)
> +{
> +    ESPState *s = container_of(d, ESPState, busdev.qdev);
> +
> +    qemu_irq_lower(s->irq);
>
> Shouldn't it be esp_lower_irq(s)? What's going to happen to the
> DMA_INTR bit if dma was the source of the irq?

Again, the registers are zeroed in esp_hard_reset().

Thanks for testing.



[Qemu-devel] Re: [PATCH] sparc32 esp fix spurious interrupts in chip reset

2010-06-01 Thread Artyom Tarasenko
2010/6/1 Blue Swirl :
> On Sun, May 30, 2010 at 10:35 PM, Artyom Tarasenko
>  wrote:
>> lower interrupt during chip reset. Otherwise the ESP_RSTAT register
>> may get out of sync with the IRQ line status. This effect became
>> visible after commit 65899fe3
>
> Hard reset handlers should not touch qemu_irqs, because on cold start,
> the receiving end may be unprepared to handle the signal.

Wouldn't the real hardware lower irq on the hardware reset?
And if it would not, would it still clear the corresponding bit in
the ESP_RSTAT register?

> See
> 0d0a7e69e853639b123798877e019c3c7ee6634a,
> bc26e55a6615dc594be425d293db40d5cdcdb84b and
> 42f1ced228c9b616cfa2b69846025271618e4ef5.
>
> For ESP there are two other sources of reset: signal from DMA and chip
> reset command. On those cases, lowering IRQ makes sense.
>
> So the correct fix is to refactor the reset handling a bit. Does this
> patch also fix your test case?

It does, but

+static void esp_soft_reset(DeviceState *d)
+{
+ESPState *s = container_of(d, ESPState, busdev.qdev);
+
+qemu_irq_lower(s->irq);

Shouldn't it be esp_lower_irq(s)? What's going to happen to the
DMA_INTR bit if dma was the source of the irq?

+esp_hard_reset(d);
+}
+



-- 
Regards,
Artyom Tarasenko

solaris/sparc under qemu blog: http://tyom.blogspot.com/



Re: [Qemu-devel] [PATCH] Extra scan codes for missing keys

2010-06-01 Thread Anthony Liguori

On 06/01/2010 02:29 PM, Bernhard M. Wiedemann wrote:

The code comes from
http://lists.gnu.org/archive/html/qemu-devel/2010-05/msg02788.html

Without this patch it is not possible to send at least 10 special
characters (\|'"`~:;[]{}) via the monitor sendkey command.

Signed-off-by: Bernhard M. Wiedemann
---
  monitor.c |6 ++
  1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index 15b53b9..09ff8cf 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1563,6 +1563,8 @@ static const KeyDef key_defs[] = {
  { 0x17, "i" },
  { 0x18, "o" },
  { 0x19, "p" },
+{ 0x1a, "sqr_brack_l" },
+{ 0x1b, "sqr_brack_r" },
   


I think it would be better to spell it out as: bracket_left and 
bracket_right


Regards,

Anthony Liguori



  { 0x1c, "ret" },

@@ -1575,7 +1577,11 @@ static const KeyDef key_defs[] = {
  { 0x24, "j" },
  { 0x25, "k" },
  { 0x26, "l" },
+{ 0x27, "semicolon" },
+{ 0x28, "apostrophe" },
+{ 0x29, "grave_accent" },

+{ 0x2b, "backslash" },
  { 0x2c, "z" },
  { 0x2d, "x" },
  { 0x2e, "c" },
   





[Qemu-devel] [PATCH] Extra scan codes for missing keys

2010-06-01 Thread Bernhard M. Wiedemann
The code comes from
http://lists.gnu.org/archive/html/qemu-devel/2010-05/msg02788.html

Without this patch it is not possible to send at least 10 special
characters (\|'"`~:;[]{}) via the monitor sendkey command.

Signed-off-by: Bernhard M. Wiedemann 
---
 monitor.c |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index 15b53b9..09ff8cf 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1563,6 +1563,8 @@ static const KeyDef key_defs[] = {
 { 0x17, "i" },
 { 0x18, "o" },
 { 0x19, "p" },
+{ 0x1a, "sqr_brack_l" },
+{ 0x1b, "sqr_brack_r" },
 
 { 0x1c, "ret" },
 
@@ -1575,7 +1577,11 @@ static const KeyDef key_defs[] = {
 { 0x24, "j" },
 { 0x25, "k" },
 { 0x26, "l" },
+{ 0x27, "semicolon" },
+{ 0x28, "apostrophe" },
+{ 0x29, "grave_accent" },
 
+{ 0x2b, "backslash" },
 { 0x2c, "z" },
 { 0x2d, "x" },
 { 0x2e, "c" },
-- 
1.5.6.5



Re: [Qemu-devel] [PATCH 1/3] monitor: Reorder info documentation

2010-06-01 Thread Anthony Liguori

On 05/31/2010 12:43 PM, Luiz Capitulino wrote:

From: Jan Kiszka

Push the doc fragments for the info command to the end of
qemu-monitor.hx. This helps to establish a proper layout in the upcoming
QMP documentation.

Signed-off-by: Jan Kiszka
   


Applied all.  Thanks.

Regards,

Anthony Liguori


---
  qemu-monitor.hx |  186 --
  1 files changed, 96 insertions(+), 90 deletions(-)

diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index f4f88df..b8c765b 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -38,96 +38,6 @@ Commit changes to the disk images (if -snapshot is used) or 
backing files.
  ETEXI

  {
-.name   = "info",
-.args_type  = "item:s?",
-.params = "[subcommand]",
-.help   = "show various information about the system state",
-.user_print = monitor_user_noop,
-.mhandler.cmd_new = do_info,
-},
-
-STEXI
-...@item info @var{subcommand}
-...@findex info
-Show various information about the system state.
-
-...@table @option
-...@item info version
-show the version of QEMU
-...@item info commands
-list QMP available commands
-...@item info network
-show the various VLANs and the associated devices
-...@item info chardev
-show the character devices
-...@item info block
-show the block devices
-...@item info blockstats
-show block device statistics
-...@item info registers
-show the cpu registers
-...@item info cpus
-show infos for each CPU
-...@item info history
-show the command line history
-...@item info irq
-show the interrupts statistics (if available)
-...@item info pic
-show i8259 (PIC) state
-...@item info pci
-show emulated PCI device info
-...@item info tlb
-show virtual to physical memory mappings (i386 only)
-...@item info mem
-show the active virtual memory mappings (i386 only)
-...@item info hpet
-show state of HPET (i386 only)
-...@item info jit
-show dynamic compiler info
-...@item info kvm
-show KVM information
-...@item info numa
-show NUMA information
-...@item info usb
-show USB devices plugged on the virtual USB hub
-...@item info usbhost
-show all USB host devices
-...@item info profile
-show profiling information
-...@item info capture
-show information about active capturing
-...@item info snapshots
-show list of VM snapshots
-...@item info status
-show the current VM status (running|paused)
-...@item info pcmcia
-show guest PCMCIA status
-...@item info mice
-show which guest mouse is receiving events
-...@item info vnc
-show the vnc server status
-...@item info name
-show the current VM name
-...@item info uuid
-show the current VM UUID
-...@item info cpustats
-show CPU statistics
-...@item info usernet
-show user network stack connection states
-...@item info migrate
-show migration status
-...@item info balloon
-show balloon information
-...@item info qtree
-show device tree
-...@item info qdm
-show qdev device model list
-...@item info roms
-show roms
-...@end table
-ETEXI
-
-{
  .name   = "q|quit",
  .args_type  = "",
  .params = "",
@@ -1190,6 +1100,102 @@ STEXI
  Enable the specified QMP capabilities
  ETEXI

+
+HXCOMM Keep the 'info' command at the end!
+HXCOMM This is required for the QMP documentation layout.
+
+{
+.name   = "info",
+.args_type  = "item:s?",
+.params = "[subcommand]",
+.help   = "show various information about the system state",
+.user_print = monitor_user_noop,
+.mhandler.cmd_new = do_info,
+},
+
+STEXI
+...@item info @var{subcommand}
+...@findex info
+Show various information about the system state.
+
+...@table @option
+...@item info version
+show the version of QEMU
+...@item info commands
+list QMP available commands
+...@item info network
+show the various VLANs and the associated devices
+...@item info chardev
+show the character devices
+...@item info block
+show the block devices
+...@item info blockstats
+show block device statistics
+...@item info registers
+show the cpu registers
+...@item info cpus
+show infos for each CPU
+...@item info history
+show the command line history
+...@item info irq
+show the interrupts statistics (if available)
+...@item info pic
+show i8259 (PIC) state
+...@item info pci
+show emulated PCI device info
+...@item info tlb
+show virtual to physical memory mappings (i386 only)
+...@item info mem
+show the active virtual memory mappings (i386 only)
+...@item info hpet
+show state of HPET (i386 only)
+...@item info jit
+show dynamic compiler info
+...@item info kvm
+show KVM information
+...@item info numa
+show NUMA information
+...@item info usb
+show USB devices plugged on the virtual USB hub
+...@item info usbhost
+show all USB host devices
+...@item info profile
+show profiling information
+...@item info capture
+show information about active capturing
+...@item info snapshots
+show list of VM snapshots
+...@item info status
+show the current VM status (running|paused)
+...@item info pcm

Re: [Qemu-devel] Re: [PATCH] qdev: Reject duplicate and anti-social device IDs

2010-06-01 Thread Anthony Liguori

On 06/01/2010 01:35 PM, Markus Armbruster wrote:

Luiz Capitulino  writes:

   

On Tue, 01 Jun 2010 16:44:24 +0200
Markus Armbruster  wrote:

 

Luiz Capitulino  writes:

   

On Mon, 31 May 2010 16:13:12 +0200
Markus Armbruster  wrote:

 

We need Device IDs to be unique and not contain '/' so device tree
nodes can always be unambigously referenced by tree path.

We already have some protection against duplicate IDs, but it got
holes:

* We don't assign IDs to default devices.

* -device and device_add use the ID of a qemu_device_opts.  Which
   rejects duplicate IDs.

* pci_add nic -net use either the ID or option "name" of
   qemu_net_opts.  And there's our hole.  Reproducible with "-net user
   -net nic,id=foo -device lsi,id=foo".
   

  Two bugs that might not be related to this thread:

   * "id" member is not mandatory for the device_add command:

 { "execute": "device_add", "arguments": { "driver": "e1000" } }
 {"return": {}}
 

Works as designed.
   

  What about netdev_add?

  { "execute": "netdev_add", "arguments": { "type": "user" } }
  {"error": {"class": "MissingParameter", "desc": "Parameter 'id' is missing", "data": 
{"name": "id"}}}
 

The only way to put a netdev to use is connecting it to a NIC with
-device DRIVER,netdev=ID,...  And that requires an ID.
   


To be honest, I think we should universally require an id parameter or 
we should auto-assign one and return it during creation.


Implicit/Null ids with QemuOpts are really difficult to work with and it 
certainly makes device addressing a lot easier.


Regards,

Anthony Liguori





[Qemu-devel] [PATCH 01/14] blockdev: Belatedly remove MAX_DRIVES

2010-06-01 Thread Markus Armbruster
Unused since commit 751c6a17.

Signed-off-by: Markus Armbruster 
---
 sysemu.h |1 -
 vl.c |2 --
 2 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/sysemu.h b/sysemu.h
index 879446a..063319c 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -176,7 +176,6 @@ typedef struct DriveInfo {
 
 #define MAX_IDE_DEVS   2
 #define MAX_SCSI_DEVS  7
-#define MAX_DRIVES 32
 
 extern QTAILQ_HEAD(drivelist, DriveInfo) drives;
 extern QTAILQ_HEAD(driveoptlist, DriveOpt) driveopts;
diff --git a/vl.c b/vl.c
index 7121cd0..f8d3034 100644
--- a/vl.c
+++ b/vl.c
@@ -172,8 +172,6 @@ int main(int argc, char **argv)
 
 static const char *data_dir;
 const char *bios_name = NULL;
-/* Note: drives_table[MAX_DRIVES] is a dummy block driver if none available
-   to store the VM snapshots */
 struct drivelist drives = QTAILQ_HEAD_INITIALIZER(drives);
 struct driveoptlist driveopts = QTAILQ_HEAD_INITIALIZER(driveopts);
 enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
-- 
1.6.6.1




[Qemu-devel] [PATCH 02/14] blockdev: Belatedly remove driveopts

2010-06-01 Thread Markus Armbruster
Unused since commit 9dfd7c7a.

Signed-off-by: Markus Armbruster 
---
 sysemu.h |1 -
 vl.c |1 -
 2 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/sysemu.h b/sysemu.h
index 063319c..fd83b7d 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -178,7 +178,6 @@ typedef struct DriveInfo {
 #define MAX_SCSI_DEVS  7
 
 extern QTAILQ_HEAD(drivelist, DriveInfo) drives;
-extern QTAILQ_HEAD(driveoptlist, DriveOpt) driveopts;
 
 extern DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit);
 extern DriveInfo *drive_get_by_id(const char *id);
diff --git a/vl.c b/vl.c
index f8d3034..9283469 100644
--- a/vl.c
+++ b/vl.c
@@ -173,7 +173,6 @@ int main(int argc, char **argv)
 static const char *data_dir;
 const char *bios_name = NULL;
 struct drivelist drives = QTAILQ_HEAD_INITIALIZER(drives);
-struct driveoptlist driveopts = QTAILQ_HEAD_INITIALIZER(driveopts);
 enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
 DisplayType display_type = DT_DEFAULT;
 const char* keyboard_layout = NULL;
-- 
1.6.6.1




[Qemu-devel] [PATCH 03/14] usb: Remove unused usb_device_add() parameter is_hotplug

2010-06-01 Thread Markus Armbruster
Unused since commit b3e461d3.

Signed-off-by: Markus Armbruster 
---
 vl.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/vl.c b/vl.c
index 9283469..76a9b25 100644
--- a/vl.c
+++ b/vl.c
@@ -1315,7 +1315,7 @@ static void smp_parse(const char *optarg)
 /***/
 /* USB devices */
 
-static int usb_device_add(const char *devname, int is_hotplug)
+static int usb_device_add(const char *devname)
 {
 const char *p;
 USBDevice *dev = NULL;
@@ -1367,7 +1367,7 @@ static int usb_device_del(const char *devname)
 static int usb_parse(const char *cmdline)
 {
 int r;
-r = usb_device_add(cmdline, 0);
+r = usb_device_add(cmdline);
 if (r < 0) {
 fprintf(stderr, "qemu: could not add USB device '%s'\n", cmdline);
 }
@@ -1377,7 +1377,7 @@ static int usb_parse(const char *cmdline)
 void do_usb_add(Monitor *mon, const QDict *qdict)
 {
 const char *devname = qdict_get_str(qdict, "devname");
-if (usb_device_add(devname, 1) < 0) {
+if (usb_device_add(devname) < 0) {
 error_report("could not add USB device '%s'", devname);
 }
 }
-- 
1.6.6.1




[Qemu-devel] [PATCH 09/14] qdev: New qdev_prop_set_string()

2010-06-01 Thread Markus Armbruster

Signed-off-by: Markus Armbruster 
---
 hw/qdev-properties.c |5 +
 hw/qdev.h|1 +
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 9ffdba7..b6ee50f 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -617,6 +617,11 @@ void qdev_prop_set_uint64(DeviceState *dev, const char 
*name, uint64_t value)
 qdev_prop_set(dev, name, &value, PROP_TYPE_UINT64);
 }
 
+void qdev_prop_set_string(DeviceState *dev, const char *name, char *value)
+{
+qdev_prop_set(dev, name, &value, PROP_TYPE_STRING);
+}
+
 void qdev_prop_set_drive(DeviceState *dev, const char *name, DriveInfo *value)
 {
 qdev_prop_set(dev, name, &value, PROP_TYPE_DRIVE);
diff --git a/hw/qdev.h b/hw/qdev.h
index a44060e..7c25a94 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -268,6 +268,7 @@ void qdev_prop_set_uint16(DeviceState *dev, const char 
*name, uint16_t value);
 void qdev_prop_set_uint32(DeviceState *dev, const char *name, uint32_t value);
 void qdev_prop_set_int32(DeviceState *dev, const char *name, int32_t value);
 void qdev_prop_set_uint64(DeviceState *dev, const char *name, uint64_t value);
+void qdev_prop_set_string(DeviceState *dev, const char *name, char *value);
 void qdev_prop_set_chr(DeviceState *dev, const char *name, CharDriverState 
*value);
 void qdev_prop_set_netdev(DeviceState *dev, const char *name, VLANClientState 
*value);
 void qdev_prop_set_vlan(DeviceState *dev, const char *name, VLANState *value);
-- 
1.6.6.1




Re: [Qemu-devel] [PATCH 2/2] migration: Fix calculation of bytes_transferred

2010-06-01 Thread Anthony Liguori

On 05/12/2010 08:12 AM, Pierre Riteau wrote:

When a page with all identical bytes is transferred, it is counted
as a full page (TARGET_PAGE_SIZE) although only one byte is actually
sent. Fix this by changing ram_save_block() to return the number of
bytes sent instead of a boolean value. This makes bandwidth
estimation, and consequently downtime estimation, more precise.

Signed-off-by: Pierre Riteau
   


Applied.  Thanks.

Regards,

Anthony Liguori


---
  arch_init.c |   21 -
  1 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index cf6b7b0..76317af 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -108,7 +108,7 @@ static int ram_save_block(QEMUFile *f)
  static ram_addr_t current_addr = 0;
  ram_addr_t saved_addr = current_addr;
  ram_addr_t addr = 0;
-int found = 0;
+int bytes_sent = 0;

  while (addr<  last_ram_offset) {
  if (cpu_physical_memory_get_dirty(current_addr, 
MIGRATION_DIRTY_FLAG)) {
@@ -123,19 +123,20 @@ static int ram_save_block(QEMUFile *f)
  if (is_dup_page(p, *p)) {
  qemu_put_be64(f, current_addr | RAM_SAVE_FLAG_COMPRESS);
  qemu_put_byte(f, *p);
+bytes_sent = 1;
  } else {
  qemu_put_be64(f, current_addr | RAM_SAVE_FLAG_PAGE);
  qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
+bytes_sent = TARGET_PAGE_SIZE;
  }

-found = 1;
  break;
  }
  addr += TARGET_PAGE_SIZE;
  current_addr = (saved_addr + addr) % last_ram_offset;
  }

-return found;
+return bytes_sent;
  }

  static uint64_t bytes_transferred;
@@ -206,11 +207,11 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, 
void *opaque)
  bwidth = qemu_get_clock_ns(rt_clock);

  while (!qemu_file_rate_limit(f)) {
-int ret;
+int bytes_sent;

-ret = ram_save_block(f);
-bytes_transferred += ret * TARGET_PAGE_SIZE;
-if (ret == 0) { /* no more blocks */
+bytes_sent = ram_save_block(f);
+bytes_transferred += bytes_sent;
+if (bytes_sent == 0) { /* no more blocks */
  break;
  }
  }
@@ -226,9 +227,11 @@ int ram_save_live(Monitor *mon, QEMUFile *f, int stage, 
void *opaque)

  /* try transferring iterative blocks of memory */
  if (stage == 3) {
+int bytes_sent;
+
  /* flush all remaining blocks regardless of rate limiting */
-while (ram_save_block(f) != 0) {
-bytes_transferred += TARGET_PAGE_SIZE;
+while ((bytes_sent = ram_save_block(f)) != 0) {
+bytes_transferred += bytes_sent;
  }
  cpu_physical_memory_set_dirty_tracking(0);
  }
   





[Qemu-devel] [PATCH 00/14] Block-related fixes and cleanups

2010-06-01 Thread Markus Armbruster
I'm working on cleanly separating block device host and guest parts.
I'd like to route all this work through Kevin's block tree.  This is
just preliminaries.

v2: Don't break IDE serial

Markus Armbruster (14):
  blockdev: Belatedly remove MAX_DRIVES
  blockdev: Belatedly remove driveopts
  usb: Remove unused usb_device_add() parameter is_hotplug
  ide: Remove useless IDEDeviceInfo members unit, drive
  ide: Remove redundant IDEState member conf
  ide: Split ide_init1() off ide_init2()
  ide: Change ide_init_drive() to require valid dinfo argument
  ide: Split non-qdev code off ide_init2()
  qdev: New qdev_prop_set_string()
  qdev: Don't leak string property value on hot unplug
  ide: Turn drive serial into a qdev property ide-drive.serial
  ide: Fix info qtree for ide-drive.ver
  scsi: Turn drive serial into a qdev property scsi-disk.serial
  scsi: Fix info qtree for scsi-disk.ver

 hw/ide/cmd646.c  |4 +-
 hw/ide/core.c|  106 +-
 hw/ide/internal.h|   13 +++---
 hw/ide/isa.c |2 +-
 hw/ide/macio.c   |2 +-
 hw/ide/microdrive.c  |3 +-
 hw/ide/mmio.c|2 +-
 hw/ide/piix.c|4 +-
 hw/ide/qdev.c|   20 -
 hw/qdev-properties.c |   11 +
 hw/qdev.c|6 +++
 hw/qdev.h|2 +
 hw/scsi-disk.c   |   24 ---
 sysemu.h |2 -
 vl.c |9 +---
 15 files changed, 135 insertions(+), 75 deletions(-)




[Qemu-devel] [PATCH 05/14] ide: Remove redundant IDEState member conf

2010-06-01 Thread Markus Armbruster
Commit 428c149b added IDEState member conf to let commit 0009baf1 find
the BlockConf from there.  It exists only for qdev drives, created via
ide_drive_initfn(), not for drives created via ide_init2().

But for a qdev drive, we can just as well reach its IDEDevice, which
contains the BlockConf.  Do that, and revert the parts of commit
428c149b that add IDEState member conf.

Signed-off-by: Markus Armbruster 
---
 hw/ide/core.c |   16 +++-
 hw/ide/internal.h |4 +---
 hw/ide/qdev.c |3 +--
 3 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 066fecb..c3334b1 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -98,6 +98,7 @@ static void ide_identify(IDEState *s)
 {
 uint16_t *p;
 unsigned int oldsize;
+IDEDevice *dev;
 
 if (s->identify_set) {
memcpy(s->io_buffer, s->identify_data, sizeof(s->identify_data));
@@ -165,8 +166,9 @@ static void ide_identify(IDEState *s)
 put_le16(p + 101, s->nb_sectors >> 16);
 put_le16(p + 102, s->nb_sectors >> 32);
 put_le16(p + 103, s->nb_sectors >> 48);
-if (s->conf && s->conf->physical_block_size)
-put_le16(p + 106, 0x6000 | get_physical_block_exp(s->conf));
+dev = s->unit ? s->bus->slave : s->bus->master;
+if (dev && dev->conf.physical_block_size)
+put_le16(p + 106, 0x6000 | get_physical_block_exp(&dev->conf));
 
 memcpy(s->identify_data, p, sizeof(s->identify_data));
 s->identify_set = 1;
@@ -2594,8 +2596,7 @@ void ide_bus_reset(IDEBus *bus)
 ide_clear_hob(bus);
 }
 
-void ide_init_drive(IDEState *s, DriveInfo *dinfo, BlockConf *conf,
-const char *version)
+void ide_init_drive(IDEState *s, DriveInfo *dinfo, const char *version)
 {
 int cylinders, heads, secs;
 uint64_t nb_sectors;
@@ -2620,9 +2621,6 @@ void ide_init_drive(IDEState *s, DriveInfo *dinfo, 
BlockConf *conf,
 }
 strncpy(s->drive_serial_str, drive_get_serial(s->bs),
 sizeof(s->drive_serial_str));
-if (conf) {
-s->conf = conf;
-}
 }
 if (strlen(s->drive_serial_str) == 0)
 snprintf(s->drive_serial_str, sizeof(s->drive_serial_str),
@@ -2653,9 +2651,9 @@ void ide_init2(IDEBus *bus, DriveInfo *hd0, DriveInfo 
*hd1,
 s->sector_write_timer = qemu_new_timer(vm_clock,
ide_sector_write_timer_cb, s);
 if (i == 0)
-ide_init_drive(s, hd0, NULL, NULL);
+ide_init_drive(s, hd0, NULL);
 if (i == 1)
-ide_init_drive(s, hd1, NULL, NULL);
+ide_init_drive(s, hd1, NULL);
 }
 bus->irq = irq;
 }
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index b4554ce..cf71019 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -398,7 +398,6 @@ struct IDEState {
 /* set for lba48 access */
 uint8_t lba48;
 BlockDriverState *bs;
-BlockConf *conf;
 char version[9];
 /* ATAPI specific */
 uint8_t sense_key;
@@ -555,8 +554,7 @@ uint32_t ide_data_readw(void *opaque, uint32_t addr);
 void ide_data_writel(void *opaque, uint32_t addr, uint32_t val);
 uint32_t ide_data_readl(void *opaque, uint32_t addr);
 
-void ide_init_drive(IDEState *s, DriveInfo *dinfo, BlockConf *conf,
-const char *version);
+void ide_init_drive(IDEState *s, DriveInfo *dinfo, const char *version);
 void ide_init2(IDEBus *bus, DriveInfo *hd0, DriveInfo *hd1,
qemu_irq irq);
 void ide_init_ioport(IDEBus *bus, int iobase, int iobase2);
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index b18693d..9ebb906 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -99,8 +99,7 @@ typedef struct IDEDrive {
 static int ide_drive_initfn(IDEDevice *dev)
 {
 IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);
-ide_init_drive(bus->ifs + dev->unit, dev->conf.dinfo, &dev->conf,
-   dev->version);
+ide_init_drive(bus->ifs + dev->unit, dev->conf.dinfo, dev->version);
 return 0;
 }
 
-- 
1.6.6.1




[Qemu-devel] [PATCH 10/14] qdev: Don't leak string property value on hot unplug

2010-06-01 Thread Markus Armbruster
parse_string() qemu_strdup()s the property value.  It is never freed.
It needs to be freed along with the device.  Otherwise, the value of
scsi-disk property "ver" gets leaked when hot-unplugging the disk, for
instance.

Call new PropertyInfo method free() from qdev_free().  Implement it
for qdev_prop_string.

Signed-off-by: Markus Armbruster 
---
 hw/qdev-properties.c |6 ++
 hw/qdev.c|6 ++
 hw/qdev.h|1 +
 3 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index b6ee50f..48a6b45 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -260,6 +260,11 @@ static int parse_string(DeviceState *dev, Property *prop, 
const char *str)
 return 0;
 }
 
+static void free_string(DeviceState *dev, Property *prop)
+{
+qemu_free(*(char **)qdev_get_prop_ptr(dev, prop));
+}
+
 static int print_string(DeviceState *dev, Property *prop, char *dest, size_t 
len)
 {
 char **ptr = qdev_get_prop_ptr(dev, prop);
@@ -274,6 +279,7 @@ PropertyInfo qdev_prop_string = {
 .size  = sizeof(char*),
 .parse = parse_string,
 .print = print_string,
+.free  = free_string,
 };
 
 /* --- drive --- */
diff --git a/hw/qdev.c b/hw/qdev.c
index af17486..09ff6fc 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -334,6 +334,7 @@ void qdev_init_nofail(DeviceState *dev)
 void qdev_free(DeviceState *dev)
 {
 BusState *bus;
+Property *prop;
 
 if (dev->state == DEV_STATE_INITIALIZED) {
 while (dev->num_child_bus) {
@@ -349,6 +350,11 @@ void qdev_free(DeviceState *dev)
 }
 qemu_unregister_reset(qdev_reset, dev);
 QLIST_REMOVE(dev, sibling);
+for (prop = dev->info->props; prop && prop->name; prop++) {
+if (prop->info->free) {
+prop->info->free(dev, prop);
+}
+}
 qemu_free(dev);
 }
 
diff --git a/hw/qdev.h b/hw/qdev.h
index 7c25a94..51a24e2 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -98,6 +98,7 @@ struct PropertyInfo {
 enum PropertyType type;
 int (*parse)(DeviceState *dev, Property *prop, const char *str);
 int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len);
+void (*free)(DeviceState *dev, Property *prop);
 };
 
 typedef struct GlobalProperty {
-- 
1.6.6.1




[Qemu-devel] [PATCH 14/14] scsi: Fix info qtree for scsi-disk.ver

2010-06-01 Thread Markus Armbruster
Show the actual default value instead of  when the property has
not been set.

Signed-off-by: Markus Armbruster 
---
 hw/scsi-disk.c |7 +--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index e8c066a..a3559d1 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -462,8 +462,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, 
uint8_t *outbuf)
 }
 memcpy(&outbuf[8], "QEMU", 8);
 memset(&outbuf[32], 0, 4);
-memcpy(&outbuf[32], s->version ? s->version : QEMU_VERSION,
-   MIN(4, strlen(s->version ? s->version : QEMU_VERSION)));
+memcpy(&outbuf[32], s->version, MIN(4, strlen(s->version)));
 /*
  * We claim conformance to SPC-3, which is required for guests
  * to ask for modern features like READ CAPACITY(16) or the
@@ -1066,6 +1065,10 @@ static int scsi_disk_initfn(SCSIDevice *dev)
 }
 }
 
+if (!s->version) {
+s->version = qemu_strdup(QEMU_VERSION);
+}
+
 if (bdrv_is_sg(s->bs)) {
 error_report("scsi-disk: unwanted /dev/sg*");
 return -1;
-- 
1.6.6.1




[Qemu-devel] [PATCH 13/14] scsi: Turn drive serial into a qdev property scsi-disk.serial

2010-06-01 Thread Markus Armbruster
It needs to be a qdev property, because it belongs to the drive's
guest part.

Bonus: info qtree now shows the serial number.

Signed-off-by: Markus Armbruster 
---
 hw/scsi-disk.c |   17 +
 1 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 4d20919..e8c066a 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -66,6 +66,7 @@ struct SCSIDiskState
 uint64_t max_lba;
 QEMUBH *bh;
 char *version;
+char *serial;
 };
 
 static SCSIDiskReq *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun)
@@ -359,9 +360,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, 
uint8_t *outbuf)
 
 case 0x80: /* Device serial number, optional */
 {
-const char *serial = req->dev->conf.dinfo->serial ?
-req->dev->conf.dinfo->serial : "0";
-int l = strlen(serial);
+int l = strlen(s->serial);
 
 if (l > req->cmd.xfer)
 l = req->cmd.xfer;
@@ -371,7 +370,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, 
uint8_t *outbuf)
 DPRINTF("Inquiry EVPD[Serial number] "
 "buffer size %zd\n", req->cmd.xfer);
 outbuf[buflen++] = l;
-memcpy(outbuf+buflen, serial, l);
+memcpy(outbuf+buflen, s->serial, l);
 buflen += l;
 break;
 }
@@ -1058,6 +1057,15 @@ static int scsi_disk_initfn(SCSIDevice *dev)
 }
 s->bs = s->qdev.conf.dinfo->bdrv;
 
+if (!s->serial) {
+if (*dev->conf.dinfo->serial) {
+/* try to fall back to value set with legacy -drive serial=... */
+s->serial = qemu_strdup(dev->conf.dinfo->serial);
+} else {
+s->serial = qemu_strdup("0");
+}
+}
+
 if (bdrv_is_sg(s->bs)) {
 error_report("scsi-disk: unwanted /dev/sg*");
 return -1;
@@ -1090,6 +1098,7 @@ static SCSIDeviceInfo scsi_disk_info = {
 .qdev.props   = (Property[]) {
 DEFINE_BLOCK_PROPERTIES(SCSIDiskState, qdev.conf),
 DEFINE_PROP_STRING("ver",  SCSIDiskState, version),
+DEFINE_PROP_STRING("serial",  SCSIDiskState, serial),
 DEFINE_PROP_END_OF_LIST(),
 },
 };
-- 
1.6.6.1




[Qemu-devel] [PATCH 12/14] ide: Fix info qtree for ide-drive.ver

2010-06-01 Thread Markus Armbruster
Show the actual default value instead of  when the property has
not been set.

Signed-off-by: Markus Armbruster 
---
 hw/ide/qdev.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 5e549d9..6231d77 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -110,6 +110,9 @@ static int ide_drive_initfn(IDEDevice *dev)
 
 ide_init_drive(s, dev->conf.dinfo, dev->version, serial);
 
+if (!dev->version) {
+dev->version = qemu_strdup(s->version);
+}
 if (!dev->serial) {
 dev->serial = qemu_strdup(s->drive_serial_str);
 }
-- 
1.6.6.1




[Qemu-devel] [PATCH 11/14] ide: Turn drive serial into a qdev property ide-drive.serial

2010-06-01 Thread Markus Armbruster
It needs to be a qdev property, because it belongs to the drive's
guest part.

Bonus: info qtree now shows the serial number.

Signed-off-by: Markus Armbruster 
---
 hw/ide/core.c |   11 ++-
 hw/ide/internal.h |4 +++-
 hw/ide/qdev.c |   16 +++-
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index d3328cd..70af1b6 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2596,7 +2596,8 @@ void ide_bus_reset(IDEBus *bus)
 ide_clear_hob(bus);
 }
 
-void ide_init_drive(IDEState *s, DriveInfo *dinfo, const char *version)
+void ide_init_drive(IDEState *s, DriveInfo *dinfo,
+const char *version, const char *serial)
 {
 int cylinders, heads, secs;
 uint64_t nb_sectors;
@@ -2618,9 +2619,9 @@ void ide_init_drive(IDEState *s, DriveInfo *dinfo, const 
char *version)
 s->is_cdrom = 1;
 bdrv_set_change_cb(s->bs, cdrom_change_cb, s);
 }
-strncpy(s->drive_serial_str, drive_get_serial(s->bs),
-sizeof(s->drive_serial_str));
-if (!*s->drive_serial_str) {
+if (serial && *serial) {
+strncpy(s->drive_serial_str, serial, sizeof(s->drive_serial_str));
+} else {
 snprintf(s->drive_serial_str, sizeof(s->drive_serial_str),
  "QM%05d", s->drive_serial);
 }
@@ -2669,7 +2670,7 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus, 
DriveInfo *hd0,
 dinfo = i == 0 ? hd0 : hd1;
 ide_init1(bus, i);
 if (dinfo) {
-ide_init_drive(&bus->ifs[i], dinfo, NULL);
+ide_init_drive(&bus->ifs[i], dinfo, NULL, dinfo->serial);
 } else {
 ide_reset(&bus->ifs[i]);
 }
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 6b0024d..eef1ee1 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -457,6 +457,7 @@ struct IDEDevice {
 uint32_t unit;
 BlockConf conf;
 char *version;
+char *serial;
 };
 
 typedef int (*ide_qdev_initfn)(IDEDevice *dev);
@@ -554,7 +555,8 @@ uint32_t ide_data_readw(void *opaque, uint32_t addr);
 void ide_data_writel(void *opaque, uint32_t addr, uint32_t val);
 uint32_t ide_data_readl(void *opaque, uint32_t addr);
 
-void ide_init_drive(IDEState *s, DriveInfo *dinfo, const char *version);
+void ide_init_drive(IDEState *s, DriveInfo *dinfo,
+const char *version, const char *serial);
 void ide_init2(IDEBus *bus, qemu_irq irq);
 void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
 DriveInfo *hd1, qemu_irq irq);
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 9ebb906..5e549d9 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -99,7 +99,20 @@ typedef struct IDEDrive {
 static int ide_drive_initfn(IDEDevice *dev)
 {
 IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);
-ide_init_drive(bus->ifs + dev->unit, dev->conf.dinfo, dev->version);
+IDEState *s = bus->ifs + dev->unit;
+const char *serial;
+
+serial = dev->serial;
+if (!serial) {
+/* try to fall back to value set with legacy -drive serial=... */
+serial = dev->conf.dinfo->serial;
+}
+
+ide_init_drive(s, dev->conf.dinfo, dev->version, serial);
+
+if (!dev->serial) {
+dev->serial = qemu_strdup(s->drive_serial_str);
+}
 return 0;
 }
 
@@ -111,6 +124,7 @@ static IDEDeviceInfo ide_drive_info = {
 DEFINE_PROP_UINT32("unit", IDEDrive, dev.unit, -1),
 DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf),
 DEFINE_PROP_STRING("ver",  IDEDrive, dev.version),
+DEFINE_PROP_STRING("serial",  IDEDrive, dev.serial),
 DEFINE_PROP_END_OF_LIST(),
 }
 };
-- 
1.6.6.1




[Qemu-devel] [PATCH 06/14] ide: Split ide_init1() off ide_init2()

2010-06-01 Thread Markus Armbruster

Signed-off-by: Markus Armbruster 
---
 hw/ide/core.c |   32 +---
 1 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index c3334b1..443ff10 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2633,27 +2633,29 @@ void ide_init_drive(IDEState *s, DriveInfo *dinfo, 
const char *version)
 ide_reset(s);
 }
 
+static void ide_init1(IDEBus *bus, int unit, DriveInfo *dinfo)
+{
+static int drive_serial = 1;
+IDEState *s = &bus->ifs[unit];
+
+s->bus = bus;
+s->unit = unit;
+s->drive_serial = drive_serial++;
+s->io_buffer = qemu_blockalign(s->bs, IDE_DMA_BUF_SECTORS*512 + 4);
+s->io_buffer_total_len = IDE_DMA_BUF_SECTORS*512 + 4;
+s->smart_selftest_data = qemu_blockalign(s->bs, 512);
+s->sector_write_timer = qemu_new_timer(vm_clock,
+   ide_sector_write_timer_cb, s);
+ide_init_drive(s, dinfo, NULL);
+}
+
 void ide_init2(IDEBus *bus, DriveInfo *hd0, DriveInfo *hd1,
qemu_irq irq)
 {
-IDEState *s;
-static int drive_serial = 1;
 int i;
 
 for(i = 0; i < 2; i++) {
-s = bus->ifs + i;
-s->bus = bus;
-s->unit = i;
-s->drive_serial = drive_serial++;
-s->io_buffer = qemu_blockalign(s->bs, IDE_DMA_BUF_SECTORS*512 + 4);
-s->io_buffer_total_len = IDE_DMA_BUF_SECTORS*512 + 4;
-s->smart_selftest_data = qemu_blockalign(s->bs, 512);
-s->sector_write_timer = qemu_new_timer(vm_clock,
-   ide_sector_write_timer_cb, s);
-if (i == 0)
-ide_init_drive(s, hd0, NULL);
-if (i == 1)
-ide_init_drive(s, hd1, NULL);
+ide_init1(bus, i, i == 0 ? hd0 : hd1);
 }
 bus->irq = irq;
 }
-- 
1.6.6.1




[Qemu-devel] [PATCH 08/14] ide: Split non-qdev code off ide_init2()

2010-06-01 Thread Markus Armbruster

Signed-off-by: Markus Armbruster 
---
 hw/ide/cmd646.c |4 ++--
 hw/ide/core.c   |   30 ++
 hw/ide/internal.h   |5 +++--
 hw/ide/isa.c|2 +-
 hw/ide/macio.c  |2 +-
 hw/ide/microdrive.c |3 ++-
 hw/ide/mmio.c   |2 +-
 hw/ide/piix.c   |4 ++--
 8 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index cdcc9bf..559147f 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -260,8 +260,8 @@ static int pci_cmd646_ide_initfn(PCIDevice *dev)
 irq = qemu_allocate_irqs(cmd646_set_irq, d, 2);
 ide_bus_new(&d->bus[0], &d->dev.qdev);
 ide_bus_new(&d->bus[1], &d->dev.qdev);
-ide_init2(&d->bus[0], NULL, NULL, irq[0]);
-ide_init2(&d->bus[1], NULL, NULL, irq[1]);
+ide_init2(&d->bus[0], irq[0]);
+ide_init2(&d->bus[1], irq[1]);
 
 vmstate_register(0, &vmstate_ide_pci, d);
 qemu_register_reset(cmd646_reset, d);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index cb7af38..d3328cd 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2632,7 +2632,7 @@ void ide_init_drive(IDEState *s, DriveInfo *dinfo, const 
char *version)
 ide_reset(s);
 }
 
-static void ide_init1(IDEBus *bus, int unit, DriveInfo *dinfo)
+static void ide_init1(IDEBus *bus, int unit)
 {
 static int drive_serial = 1;
 IDEState *s = &bus->ifs[unit];
@@ -2645,20 +2645,34 @@ static void ide_init1(IDEBus *bus, int unit, DriveInfo 
*dinfo)
 s->smart_selftest_data = qemu_blockalign(s->bs, 512);
 s->sector_write_timer = qemu_new_timer(vm_clock,
ide_sector_write_timer_cb, s);
-if (dinfo) {
-ide_init_drive(s, dinfo, NULL);
-} else {
-ide_reset(s);
+}
+
+void ide_init2(IDEBus *bus, qemu_irq irq)
+{
+int i;
+
+for(i = 0; i < 2; i++) {
+ide_init1(bus, i);
+ide_reset(&bus->ifs[i]);
 }
+bus->irq = irq;
 }
 
-void ide_init2(IDEBus *bus, DriveInfo *hd0, DriveInfo *hd1,
-   qemu_irq irq)
+/* TODO convert users to qdev and remove */
+void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
+DriveInfo *hd1, qemu_irq irq)
 {
 int i;
+DriveInfo *dinfo;
 
 for(i = 0; i < 2; i++) {
-ide_init1(bus, i, i == 0 ? hd0 : hd1);
+dinfo = i == 0 ? hd0 : hd1;
+ide_init1(bus, i);
+if (dinfo) {
+ide_init_drive(&bus->ifs[i], dinfo, NULL);
+} else {
+ide_reset(&bus->ifs[i]);
+}
 }
 bus->irq = irq;
 }
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index cf71019..6b0024d 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -555,8 +555,9 @@ void ide_data_writel(void *opaque, uint32_t addr, uint32_t 
val);
 uint32_t ide_data_readl(void *opaque, uint32_t addr);
 
 void ide_init_drive(IDEState *s, DriveInfo *dinfo, const char *version);
-void ide_init2(IDEBus *bus, DriveInfo *hd0, DriveInfo *hd1,
-   qemu_irq irq);
+void ide_init2(IDEBus *bus, qemu_irq irq);
+void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
+DriveInfo *hd1, qemu_irq irq);
 void ide_init_ioport(IDEBus *bus, int iobase, int iobase2);
 
 /* hw/ide/qdev.c */
diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index dff7c79..b6c6347 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -70,7 +70,7 @@ static int isa_ide_initfn(ISADevice *dev)
 ide_bus_new(&s->bus, &s->dev.qdev);
 ide_init_ioport(&s->bus, s->iobase, s->iobase2);
 isa_init_irq(dev, &s->irq, s->isairq);
-ide_init2(&s->bus, NULL, NULL, s->irq);
+ide_init2(&s->bus, s->irq);
 vmstate_register(0, &vmstate_ide_isa, s);
 return 0;
 };
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index 639f3f6..f76c0fa 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -314,7 +314,7 @@ int pmac_ide_init (DriveInfo **hd_table, qemu_irq irq,
 int pmac_ide_memory;
 
 d = qemu_mallocz(sizeof(MACIOIDEState));
-ide_init2(&d->bus, hd_table[0], hd_table[1], irq);
+ide_init2_with_non_qdev_drives(&d->bus, hd_table[0], hd_table[1], irq);
 
 if (dbdma)
 DBDMA_register_channel(dbdma, channel, dma_irq, pmac_ide_transfer, 
pmac_ide_flush, d);
diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c
index bfdb8c8..a7beac5 100644
--- a/hw/ide/microdrive.c
+++ b/hw/ide/microdrive.c
@@ -539,7 +539,8 @@ PCMCIACardState *dscm1_init(DriveInfo *bdrv)
 md->card.cis = dscm1_cis;
 md->card.cis_len = sizeof(dscm1_cis);
 
-ide_init2(&md->bus, bdrv, NULL, qemu_allocate_irqs(md_set_irq, md, 1)[0]);
+ide_init2_with_non_qdev_drives(&md->bus, bdrv, NULL,
+   qemu_allocate_irqs(md_set_irq, md, 1)[0]);
 md->bus.ifs[0].is_cf = 1;
 md->bus.ifs[0].mdata_size = METADATA_SIZE;
 md->bus.ifs[0].mdata_storage = (uint8_t *) qemu_mallocz(METADATA_SIZE);
diff --git a/hw/ide/mmio.c b/hw/ide/mmio.c
index cca883f..e75cccf 100644
--- a/hw/ide/mmio.c
+++ b

[Qemu-devel] [PATCH 07/14] ide: Change ide_init_drive() to require valid dinfo argument

2010-06-01 Thread Markus Armbruster
IDEState members drive_serial_str and version are now left empty until
an actual drive is connected.  Before, they got a default value that
was overwritten when a drive got connected.  Doesn't matter, because
they're used only while a drive is connected.

Signed-off-by: Markus Armbruster 
---
 hw/ide/core.c |   47 +--
 1 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 443ff10..cb7af38 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2601,30 +2601,29 @@ void ide_init_drive(IDEState *s, DriveInfo *dinfo, 
const char *version)
 int cylinders, heads, secs;
 uint64_t nb_sectors;
 
-if (dinfo && dinfo->bdrv) {
-s->bs = dinfo->bdrv;
-bdrv_get_geometry(s->bs, &nb_sectors);
-bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs);
-s->cylinders = cylinders;
-s->heads = heads;
-s->sectors = secs;
-s->nb_sectors = nb_sectors;
-/* The SMART values should be preserved across power cycles
-   but they aren't.  */
-s->smart_enabled = 1;
-s->smart_autosave = 1;
-s->smart_errors = 0;
-s->smart_selftest_count = 0;
-if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM) {
-s->is_cdrom = 1;
-bdrv_set_change_cb(s->bs, cdrom_change_cb, s);
-}
-strncpy(s->drive_serial_str, drive_get_serial(s->bs),
-sizeof(s->drive_serial_str));
+s->bs = dinfo->bdrv;
+bdrv_get_geometry(s->bs, &nb_sectors);
+bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs);
+s->cylinders = cylinders;
+s->heads = heads;
+s->sectors = secs;
+s->nb_sectors = nb_sectors;
+/* The SMART values should be preserved across power cycles
+   but they aren't.  */
+s->smart_enabled = 1;
+s->smart_autosave = 1;
+s->smart_errors = 0;
+s->smart_selftest_count = 0;
+if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM) {
+s->is_cdrom = 1;
+bdrv_set_change_cb(s->bs, cdrom_change_cb, s);
 }
-if (strlen(s->drive_serial_str) == 0)
+strncpy(s->drive_serial_str, drive_get_serial(s->bs),
+sizeof(s->drive_serial_str));
+if (!*s->drive_serial_str) {
 snprintf(s->drive_serial_str, sizeof(s->drive_serial_str),
  "QM%05d", s->drive_serial);
+}
 if (version) {
 pstrcpy(s->version, sizeof(s->version), version);
 } else {
@@ -2646,7 +2645,11 @@ static void ide_init1(IDEBus *bus, int unit, DriveInfo 
*dinfo)
 s->smart_selftest_data = qemu_blockalign(s->bs, 512);
 s->sector_write_timer = qemu_new_timer(vm_clock,
ide_sector_write_timer_cb, s);
-ide_init_drive(s, dinfo, NULL);
+if (dinfo) {
+ide_init_drive(s, dinfo, NULL);
+} else {
+ide_reset(s);
+}
 }
 
 void ide_init2(IDEBus *bus, DriveInfo *hd0, DriveInfo *hd1,
-- 
1.6.6.1




[Qemu-devel] Re: [PULLv2] pci fixes

2010-06-01 Thread Anthony Liguori

On 05/31/2010 01:47 PM, Michael S. Tsirkin wrote:

OK, I dropped the vhost change for now: I still think
we should not bother about mingw for linux-only code,
but at this point it's not important for me.

The following changes since commit aa6f63fff62faf2fe9ffba5a789675d49293614d:

   mc146818rtc: improve debugging (2010-05-30 19:20:07 +)

are available in the git repository at:
   git://git.kernel.org/pub/scm/linux/kernel/git/mst/qemu.git for_anthony
   


Pulled.  Thanks.

Regards,

Anthony Liguori


Isaku Yamahata (5):
   pci: clean up of pci_set_default_subsystem_id().
   pci: add const to pci_is_express(), pci_config_size().
   pci.h: remove unused constants.
   msix: remove duplicated defines.
   pci-hotplug: make them aware of pci domain.

  hw/msix.c|8 
  hw/pci-hotplug.c |7 ---
  hw/pci.c |   34 ++
  hw/pci.h |   16 +++-
  4 files changed, 33 insertions(+), 32 deletions(-)
   





[Qemu-devel] [PATCH 04/14] ide: Remove useless IDEDeviceInfo members unit, drive

2010-06-01 Thread Markus Armbruster

Signed-off-by: Markus Armbruster 
---
 hw/ide/internal.h |2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 2efc784..b4554ce 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -464,8 +464,6 @@ typedef int (*ide_qdev_initfn)(IDEDevice *dev);
 struct IDEDeviceInfo {
 DeviceInfo qdev;
 ide_qdev_initfn init;
-uint32_t unit;
-DriveInfo *drive;
 };
 
 #define BM_STATUS_DMAING 0x01
-- 
1.6.6.1




Re: [Qemu-devel] [PATCH 1/5] vnc: factor out vnc_desktop_resize()

2010-06-01 Thread Anthony Liguori

On 05/25/2010 11:25 AM, Gerd Hoffmann wrote:

Signed-off-by: Gerd Hoffmann
   


Applied all.  Thanks.

Regards,

Anthony Liguori


---
  vnc.c |   24 
  1 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/vnc.c b/vnc.c
index 11ae3e5..aaebe24 100644
--- a/vnc.c
+++ b/vnc.c
@@ -514,6 +514,21 @@ void buffer_append(Buffer *buffer, const void *data, 
size_t len)
  buffer->offset += len;
  }

+static void vnc_desktop_resize(VncState *vs)
+{
+DisplayState *ds = vs->ds;
+
+if (vs->csock == -1 || !vnc_has_feature(vs, VNC_FEATURE_RESIZE)) {
+return;
+}
+vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
+vnc_write_u8(vs, 0);
+vnc_write_u16(vs, 1); /* number of rects */
+vnc_framebuffer_update(vs, 0, 0, ds_get_width(ds), ds_get_height(ds),
+   VNC_ENCODING_DESKTOPRESIZE);
+vnc_flush(vs);
+}
+
  static void vnc_dpy_resize(DisplayState *ds)
  {
  int size_changed;
@@ -542,14 +557,7 @@ static void vnc_dpy_resize(DisplayState *ds)
  QTAILQ_FOREACH(vs,&vd->clients, next) {
  vnc_colordepth(vs);
  if (size_changed) {
-if (vs->csock != -1&&  vnc_has_feature(vs, VNC_FEATURE_RESIZE)) {
-vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
-vnc_write_u8(vs, 0);
-vnc_write_u16(vs, 1); /* number of rects */
-vnc_framebuffer_update(vs, 0, 0, ds_get_width(ds), 
ds_get_height(ds),
-VNC_ENCODING_DESKTOPRESIZE);
-vnc_flush(vs);
-}
+vnc_desktop_resize(vs);
  }
  if (vs->vd->cursor) {
  vnc_cursor_define(vs);
   





Re: [Qemu-devel] Re: [PATCH] qdev: Reject duplicate and anti-social device IDs

2010-06-01 Thread Markus Armbruster
Luiz Capitulino  writes:

> On Tue, 01 Jun 2010 16:44:24 +0200
> Markus Armbruster  wrote:
>
>> Luiz Capitulino  writes:
>> 
>> > On Mon, 31 May 2010 16:13:12 +0200
>> > Markus Armbruster  wrote:
>> >
>> >> We need Device IDs to be unique and not contain '/' so device tree
>> >> nodes can always be unambigously referenced by tree path.
>> >> 
>> >> We already have some protection against duplicate IDs, but it got
>> >> holes:
>> >> 
>> >> * We don't assign IDs to default devices.
>> >> 
>> >> * -device and device_add use the ID of a qemu_device_opts.  Which
>> >>   rejects duplicate IDs.
>> >> 
>> >> * pci_add nic -net use either the ID or option "name" of
>> >>   qemu_net_opts.  And there's our hole.  Reproducible with "-net user
>> >>   -net nic,id=foo -device lsi,id=foo".
>> >
>> >  Two bugs that might not be related to this thread:
>> >
>> >   * "id" member is not mandatory for the device_add command:
>> >
>> > { "execute": "device_add", "arguments": { "driver": "e1000" } }
>> > {"return": {}}
>> 
>> Works as designed.
>
>  What about netdev_add?
>
>  { "execute": "netdev_add", "arguments": { "type": "user" } }
>  {"error": {"class": "MissingParameter", "desc": "Parameter 'id' is missing", 
> "data": {"name": "id"}}}

The only way to put a netdev to use is connecting it to a NIC with
-device DRIVER,netdev=ID,...  And that requires an ID.



[Qemu-devel] Re: [PATCH 07/14] ide: Change ide_init_drive() to require valid dinfo argument

2010-06-01 Thread Markus Armbruster
Kevin Wolf  writes:

> Am 28.05.2010 15:38, schrieb Markus Armbruster:
>> IDEState members drive_serial_str and version are now left empty until
>> an actual drive is connected.  Before, they got a default value that
>> was overwritten when a drive got connected.  Doesn't matter, because
>> they're used only while a drive is connected.
>> 
>> Signed-off-by: Markus Armbruster 
>> ---
>>  hw/ide/core.c |   47 +--
>>  1 files changed, 25 insertions(+), 22 deletions(-)
>> 
>> diff --git a/hw/ide/core.c b/hw/ide/core.c
>> index 443ff10..f72d37f 100644
>> --- a/hw/ide/core.c
>> +++ b/hw/ide/core.c
>> @@ -2601,30 +2601,29 @@ void ide_init_drive(IDEState *s, DriveInfo *dinfo, 
>> const char *version)
>>  int cylinders, heads, secs;
>>  uint64_t nb_sectors;
>>  
>> -if (dinfo && dinfo->bdrv) {
>> -s->bs = dinfo->bdrv;
>> -bdrv_get_geometry(s->bs, &nb_sectors);
>> -bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs);
>> -s->cylinders = cylinders;
>> -s->heads = heads;
>> -s->sectors = secs;
>> -s->nb_sectors = nb_sectors;
>> -/* The SMART values should be preserved across power cycles
>> -   but they aren't.  */
>> -s->smart_enabled = 1;
>> -s->smart_autosave = 1;
>> -s->smart_errors = 0;
>> -s->smart_selftest_count = 0;
>> -if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM) {
>> -s->is_cdrom = 1;
>> -bdrv_set_change_cb(s->bs, cdrom_change_cb, s);
>> -}
>> -strncpy(s->drive_serial_str, drive_get_serial(s->bs),
>> -sizeof(s->drive_serial_str));
>> +s->bs = dinfo->bdrv;
>> +bdrv_get_geometry(s->bs, &nb_sectors);
>> +bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs);
>> +s->cylinders = cylinders;
>> +s->heads = heads;
>> +s->sectors = secs;
>> +s->nb_sectors = nb_sectors;
>> +/* The SMART values should be preserved across power cycles
>> +   but they aren't.  */
>> +s->smart_enabled = 1;
>> +s->smart_autosave = 1;
>> +s->smart_errors = 0;
>> +s->smart_selftest_count = 0;
>> +if (bdrv_get_type_hint(s->bs) == BDRV_TYPE_CDROM) {
>> +s->is_cdrom = 1;
>> +bdrv_set_change_cb(s->bs, cdrom_change_cb, s);
>>  }
>> -if (strlen(s->drive_serial_str) == 0)
>> +strncpy(s->drive_serial_str, drive_get_serial(s->bs),
>> +sizeof(s->drive_serial_str));
>> +if (*s->drive_serial_str) {
>
> This should be the other way round. It breaks the serial number in both
> cases.

I have no idea how that could get past my testing.  I'll respin.
Thanks!



Re: [Qemu-devel] [PATCH] Add support for depth 15 to qemu_default_pixelformat()

2010-06-01 Thread Anthony Liguori

On 05/21/2010 04:59 AM, Gerd Hoffmann wrote:

Makes qemu_default_pixelformat(15) return pixelformat filled for 15 bit
color depth (16 bpp, 5 bits for red,green,blue each, 1 bit unused).

Signed-off-by: Gerd Hoffmann
   


Applied.  Thanks.

Regards,

Anthony Liguori

---
  console.c |   16 
  1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/console.c b/console.c
index 7070b1b..c91e2bb 100644
--- a/console.c
+++ b/console.c
@@ -1621,6 +1621,22 @@ PixelFormat qemu_default_pixelformat(int bpp)
  pf.depth = bpp == 32 ? 24 : bpp;

  switch (bpp) {
+case 15:
+pf.bits_per_pixel = 16;
+pf.bytes_per_pixel = 2;
+pf.rmask = 0x7c00;
+pf.gmask = 0x03E0;
+pf.bmask = 0x001F;
+pf.rmax = 31;
+pf.gmax = 31;
+pf.bmax = 31;
+pf.rshift = 10;
+pf.gshift = 5;
+pf.bshift = 0;
+pf.rbits = 5;
+pf.gbits = 5;
+pf.bbits = 5;
+break;
  case 16:
  pf.rmask = 0xF800;
  pf.gmask = 0x07E0;
   





Re: [Qemu-devel] [PATCH] resent: x86/cpuid: Add kvm32 CPU model

2010-06-01 Thread Anthony Liguori

On 05/21/2010 02:50 AM, Andre Przywara wrote:

Create a kvm32 CPU model that describes a least common denominator
for KVM capable guest CPUs. Useful for migration purposes.

Signed-off-by: Andre Przywara
   


Applied.  Thanks.

Regards,

Anthony Liguori

---
  target-i386/cpuid.c |   14 ++
  1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index a80baa4..76b897d 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -363,6 +363,20 @@ static x86_def_t builtin_x86_defs[] = {
  .model_id = "QEMU Virtual CPU version " QEMU_VERSION,
  },
  {
+.name = "kvm32",
+.level = 5,
+.family = 15,
+.model = 6,
+.stepping = 1,
+.features = PPRO_FEATURES |
+CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_PSE36,
+.ext_features = CPUID_EXT_SSE3,
+.ext2_features = PPRO_FEATURES&  EXT2_FEATURE_MASK,
+.ext3_features = 0,
+.xlevel = 0x8008,
+.model_id = "Common 32-bit KVM processor"
+},
+{
  .name = "coreduo",
  .level = 10,
  .family = 6,
   





Re: [Qemu-devel] Re: [RFT][PATCH 07/15] qemu_irq: Add IRQ handlers with delivery feedback

2010-06-01 Thread Gleb Natapov
On Tue, Jun 01, 2010 at 06:00:20PM +, Blue Swirl wrote:
> >> > Looks like irr in apic is never cleared. Probably bug in userspace apic
> >> > emulation. I'll look into it. Try to route interrupt via APIC (not 
> >> > ExtInt),
> >> > or use qemu-kvm with in kernel irq chip.
> >>
> >> With APIC you mean Fixed? Then the IRQ is not delivered at all.
> > You need to deliver it through IOAPIC.
> 
> In this version, when USE_APIC is defined, IRQs are routed via IOAPIC
> and APIC, PIC interrupts are disabled. Otherwise, IOAPIC and APIC are
> left to reset state and PIC is used.
> 
> With the PIC version, there are bogus injections. With IOAPIC and APIC
> dots come at 2Hz. -enable-kvm has no effect.

I looked into this briefly. Virtual wire case is not handled correctly,
and pic lacks coalescing detection at all (the reason BTW because proper
solution was rejected, so only hack required by Windows was added).
Windows routes RTC interrupts through IOAPIC and this case works as you
can see. -enable-kvm on qemu should not make any difference since it
uses the same userspace pic/ioapic/apic code as TCG. It would be
interesting to try with qemu-kvm which uses in-kernel irq chips by
default.

BTW I managed to compile you test program with gcc 4.2. 4.4 and 4.3
produced non-working binaries for me.

--
Gleb.



Re: [Qemu-devel] [PATCH] check for active_console before using it

2010-06-01 Thread Anthony Liguori

On 05/20/2010 08:23 AM, Gerd Hoffmann wrote:

Other vga_hw_* functions do the same.
Fixes a segmentation fault.  Trigger: boot with -nodefaults,
then connect via vnc.

Signed-off-by: Gerd Hoffmann
   


Applied.  Thanks.

Regards,

Anthony Liguori

---
  console.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/console.c b/console.c
index 7070b1b..4c42b28 100644
--- a/console.c
+++ b/console.c
@@ -167,7 +167,7 @@ void vga_hw_update(void)

  void vga_hw_invalidate(void)
  {
-if (active_console->hw_invalidate)
+if (active_console&&  active_console->hw_invalidate)
  active_console->hw_invalidate(active_console->hw);
  }

   





Re: [Qemu-devel] [PATCH] Add dependency of JSON unit tests on config-host.h

2010-06-01 Thread Anthony Liguori

On 05/20/2010 02:18 AM, Jan Kiszka wrote:

From: Jan Kiszka

Signed-off-by: Jan Kiszka
   


Applied.  Thanks.

Regards,

Anthony Liguori


---
  Makefile |2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 110698e..aa81d9b 100644
--- a/Makefile
+++ b/Makefile
@@ -144,6 +144,8 @@ qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o 
$(block-obj-y) $(qobj
  qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
$(call quiet-command,sh $(SRC_PATH)/hxtool -h<  $<  >  $@,"  GEN   $@")

+check-qint.o check-qstring.o check-qdict.o check-qlist.o check-qfloat.o 
check-qjson.o: $(GENERATED_HEADERS)
+
  check-qint: check-qint.o qint.o qemu-malloc.o
  check-qstring: check-qstring.o qstring.o qemu-malloc.o
  check-qdict: check-qdict.o qdict.o qfloat.o qint.o qstring.o qbool.o 
qemu-malloc.o qlist.o
   





Re: [Qemu-devel] [PATCH] virtio-serial-bus: fix ports_map allocation on init

2010-06-01 Thread Anthony Liguori

On 05/19/2010 04:31 AM, Amit Shah wrote:

From: Alon Levy

Fix for too small allocation to ports_map

Signed-off-by: Alon Levy
Signed-off-by: Amit Shah
   


Applied.  Thanks.

Regards,

Anthony Liguori


---
  hw/virtio-serial-bus.c |3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 3ce95e8..7f9d28f 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -774,7 +774,8 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, uint32_t 
max_nr_ports)
  }

  vser->config.max_nr_ports = max_nr_ports;
-vser->ports_map = qemu_mallocz((max_nr_ports + 31) / 32);
+vser->ports_map = qemu_mallocz(((max_nr_ports + 31) / 32)
+* sizeof(vser->ports_map[0]));
  /*
   * Reserve location 0 for a console port for backward compat
   * (old kernel, new qemu)
   





Re: [Qemu-devel] [PATCH v3 01/12] Revert "vnc: set the right prefered encoding"

2010-06-01 Thread Anthony Liguori

On 05/19/2010 02:24 AM, Corentin Chary wrote:

This patch was wrong, because the loop was already reversed,
so the first encoding was correctly set at the end of the loopp.

This reverts commit 14eb8b6829ad9dee7035de729e083844a425f274.

Signed-off-by: Corentin Chary
   


Applied all.  Thanks.

Regards,

Anthony Liguori


---
  vnc.c |   14 --
  1 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/vnc.c b/vnc.c
index 1f7ad73..b1a3fdb 100644
--- a/vnc.c
+++ b/vnc.c
@@ -1594,7 +1594,7 @@ static void set_encodings(VncState *vs, int32_t 
*encodings, size_t n_encodings)

  vnc_zlib_init(vs);
  vs->features = 0;
-vs->vnc_encoding = -1;
+vs->vnc_encoding = 0;
  vs->tight_compression = 9;
  vs->tight_quality = 9;
  vs->absolute = -1;
@@ -1603,24 +1603,18 @@ static void set_encodings(VncState *vs, int32_t 
*encodings, size_t n_encodings)
  enc = encodings[i];
  switch (enc) {
  case VNC_ENCODING_RAW:
-if (vs->vnc_encoding != -1) {
-vs->vnc_encoding = enc;
-}
+vs->vnc_encoding = enc;
  break;
  case VNC_ENCODING_COPYRECT:
  vs->features |= VNC_FEATURE_COPYRECT_MASK;
  break;
  case VNC_ENCODING_HEXTILE:
  vs->features |= VNC_FEATURE_HEXTILE_MASK;
-if (vs->vnc_encoding != -1) {
-vs->vnc_encoding = enc;
-}
+vs->vnc_encoding = enc;
  break;
  case VNC_ENCODING_ZLIB:
  vs->features |= VNC_FEATURE_ZLIB_MASK;
-if (vs->vnc_encoding != -1) {
-vs->vnc_encoding = enc;
-}
+vs->vnc_encoding = enc;
  break;
  case VNC_ENCODING_DESKTOPRESIZE:
  vs->features |= VNC_FEATURE_RESIZE_MASK;
   





Re: [Qemu-devel] Re: [PATCH 2/3] QMP: Introduce commands documentation

2010-06-01 Thread Jan Kiszka
Anthony Liguori wrote:
> On 06/01/2010 12:39 PM, Jan Kiszka wrote:
>> Oh, it's indeed a local issue. Likely our corporate mail server. Nice...
>> Sorry for the noise!
>>
> 
> Jan, I believe you're okay with the current state of this series, yes?

Yes, of course!

> 
> I'd like to apply these ASAP so we can start formal spec review.

Great.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



[Qemu-devel] [PATCH v2 0/2] basic machine opts framework

2010-06-01 Thread Glauber Costa
Hello,

This is a resent (rebased) of an old patch set of mine, I sent
some time ago. With that, we should have all the needed infrastructure
to select the in-kernel irqchip for KVM.


Glauber Costa (2):
  early set current_machine
  basic machine opts framework

 hw/boards.h |   10 +
 hw/pc.c |   45 -
 qemu-config.c   |   16 ++
 qemu-config.h   |1 +
 qemu-options.hx |9 
 vl.c|   59 +-
 6 files changed, 132 insertions(+), 8 deletions(-)




Re: [Qemu-devel] Re: [PATCH 2/3] QMP: Introduce commands documentation

2010-06-01 Thread Anthony Liguori

On 06/01/2010 12:39 PM, Jan Kiszka wrote:


Oh, it's indeed a local issue. Likely our corporate mail server. Nice...
Sorry for the noise!
   


Jan, I believe you're okay with the current state of this series, yes?

I'd like to apply these ASAP so we can start formal spec review.

Regards,

Anthony Liguori


Jan

   





[Qemu-devel] [PATCH v2 2/2] basic machine opts framework

2010-06-01 Thread Glauber Costa
This patch adds initial support for the -machine option, that allows
command line specification of machine attributes (always relying on safe
defaults). Besides its value per-se, it is the saner way we found to
allow for enabling/disabling of kvm's in-kernel irqchip.

A machine with in-kernel-irqchip could be specified as:
-machine irqchip=apic-kvm
And one without it:
-machine irqchip=apic

To demonstrate how it'd work, this patch introduces a choice between
"pic" and "apic", pic being the old-style isa thing.
---
 hw/boards.h |   10 ++
 hw/pc.c |   45 +++--
 qemu-config.c   |   16 
 qemu-config.h   |1 +
 qemu-options.hx |9 +
 vl.c|   54 ++
 6 files changed, 129 insertions(+), 6 deletions(-)

diff --git a/hw/boards.h b/hw/boards.h
index d889341..187794e 100644
--- a/hw/boards.h
+++ b/hw/boards.h
@@ -12,6 +12,15 @@ typedef void QEMUMachineInitFunc(ram_addr_t ram_size,
  const char *initrd_filename,
  const char *cpu_model);
 
+typedef void (QEMUIrqchipFunc)(void *opaque);
+
+typedef struct QEMUIrqchip {
+const char *name;
+QEMUIrqchipFunc *init; 
+int used;
+int is_default;
+} QEMUIrqchip;
+
 typedef struct QEMUMachine {
 const char *name;
 const char *alias;
@@ -21,6 +30,7 @@ typedef struct QEMUMachine {
 int max_cpus;
 int is_default;
 CompatProperty *compat_props;
+QEMUIrqchip *irqchip;
 struct QEMUMachine *next;
 } QEMUMachine;
 
diff --git a/hw/pc.c b/hw/pc.c
index 408d6d6..b3de30a 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1007,21 +1007,43 @@ int cpu_is_bsp(CPUState *env)
 return env->cpuid_apic_id == 0;
 }
 
+static void qemu_apic_init(void *opaque)
+{
+CPUState *env = opaque;
+if (!(env->cpuid_features & CPUID_APIC)) {
+fprintf(stderr, "CPU lacks APIC cpuid flag\n");
+exit(1);
+}
+env->cpuid_apic_id = env->cpu_index;
+/* APIC reset callback resets cpu */
+apic_init(env);
+}
+
+static void qemu_pic_init(void *opaque)
+{
+CPUState *env = opaque;
+
+if (smp_cpus > 1) {
+fprintf(stderr, "PIC can't support smp systems\n");
+exit(1);
+}
+qemu_register_reset((QEMUResetHandler*)cpu_reset, env);
+}
+
 static CPUState *pc_new_cpu(const char *cpu_model)
 {
 CPUState *env;
+QEMUIrqchip *ic;
 
 env = cpu_init(cpu_model);
 if (!env) {
 fprintf(stderr, "Unable to find x86 CPU definition\n");
 exit(1);
 }
-if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) {
-env->cpuid_apic_id = env->cpu_index;
-/* APIC reset callback resets cpu */
-apic_init(env);
-} else {
-qemu_register_reset((QEMUResetHandler*)cpu_reset, env);
+
+for (ic = current_machine->irqchip; ic->name != NULL; ic++) {
+if (ic->used)
+ic->init(env);
 }
 return env;
 }
@@ -1370,6 +1392,17 @@ static QEMUMachine pc_machine = {
 .desc = "Standard PC",
 .init = pc_init_pci,
 .max_cpus = 255,
+.irqchip = (QEMUIrqchip[]){
+{
+.name = "apic",
+.init = qemu_apic_init,
+.is_default = 1,
+},{
+.name = "pic",
+.init = qemu_pic_init,
+},
+{ /* end of list */ },
+},
 .is_default = 1,
 };
 
diff --git a/qemu-config.c b/qemu-config.c
index cae92f7..e83b301 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -196,6 +196,21 @@ QemuOptsList qemu_rtc_opts = {
 },
 };
 
+QemuOptsList qemu_machine_opts = {
+.name = "M",
+.head = QTAILQ_HEAD_INITIALIZER(qemu_machine_opts.head),
+.desc = {
+{
+.name = "mach",
+.type = QEMU_OPT_STRING,
+},{
+.name = "irqchip",
+.type = QEMU_OPT_STRING,
+},
+{ /* end of list */ }
+},
+};
+
 static QemuOptsList *lists[] = {
 &qemu_drive_opts,
 &qemu_chardev_opts,
@@ -203,6 +218,7 @@ static QemuOptsList *lists[] = {
 &qemu_netdev_opts,
 &qemu_net_opts,
 &qemu_rtc_opts,
+&qemu_machine_opts,
 NULL,
 };
 
diff --git a/qemu-config.h b/qemu-config.h
index 3cc8864..9b02ee0 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -7,6 +7,7 @@ extern QemuOptsList qemu_device_opts;
 extern QemuOptsList qemu_netdev_opts;
 extern QemuOptsList qemu_net_opts;
 extern QemuOptsList qemu_rtc_opts;
+extern QemuOptsList qemu_machine_opts;
 
 int qemu_set_option(const char *str);
 
diff --git a/qemu-options.hx b/qemu-options.hx
index 20aa242..4dfc55a 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -31,6 +31,15 @@ STEXI
 Select the emulated @var{machine} (@code{-M ?} for list)
 ETEXI
 
+DEF("machine", HAS_ARG, QEMU_OPTION_machine,
+"-machine mach=m[,irqchip=chip]\n"
+"select emulated machine (-machine ? for list)\n")
+STEXI
+...@item -machine @var{machine}[,@var{op

[Qemu-devel] [PATCH v2 1/2] early set current_machine

2010-06-01 Thread Glauber Costa
this way, the machine_init function itself can know which machine is current
in use, not only the late init code.
Signed-off-by: Glauber Costa 
---
 vl.c |5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index 96838f8..7a8b20b 100644
--- a/vl.c
+++ b/vl.c
@@ -5824,6 +5824,9 @@ int main(int argc, char **argv, char **envp)
 if (machine->compat_props) {
 qdev_prop_register_compat(machine->compat_props);
 }
+
+current_machine = machine;
+
 machine->init(ram_size, boot_devices,
   kernel_filename, kernel_cmdline, initrd_filename, cpu_model);
 
@@ -5841,8 +5844,6 @@ int main(int argc, char **argv, char **envp)
 }
 }
 
-current_machine = machine;
-
 /* init USB devices */
 if (usb_enabled) {
 if (foreach_device_config(DEV_USB, usb_parse) < 0)
-- 
1.6.2.2




Re: [Qemu-devel] Re: [RFT][PATCH 07/15] qemu_irq: Add IRQ handlers with delivery feedback

2010-06-01 Thread Blue Swirl
On Mon, May 31, 2010 at 5:19 AM, Gleb Natapov  wrote:
> On Sun, May 30, 2010 at 08:21:30PM +, Blue Swirl wrote:
>> On Sun, May 30, 2010 at 8:07 PM, Gleb Natapov  wrote:
>> > On Sun, May 30, 2010 at 07:37:59PM +, Blue Swirl wrote:
>> >> On Sun, May 30, 2010 at 1:49 PM, Gleb Natapov  wrote:
>> >> > On Sun, May 30, 2010 at 12:56:26PM +, Blue Swirl wrote:
>> >> >> >> Well, I'd like to get the test program also trigger it. Now I'm 
>> >> >> >> getting:
>> >> >> >> apic: write: 0350 = 
>> >> >> >> apic: apic_reset_irq_delivered: old coalescing 0
>> >> >> >> apic: apic_local_deliver: vector 3 delivery mode 0
>> >> >> >> apic: apic_set_irq: coalescing 1
>> >> >> >> apic: apic_get_irq_delivered: returning coalescing 1
>> >> >> >> apic: apic_reset_irq_delivered: old coalescing 1
>> >> >> >> apic: apic_local_deliver: vector 3 delivery mode 0
>> >> >> >> apic: apic_set_irq: coalescing 0
>> >> >> >> apic: apic_get_irq_delivered: returning coalescing 0
>> >> >> >> apic: apic_reset_irq_delivered: old coalescing 0
>> >> >> >> apic: apic_local_deliver: vector 3 delivery mode 0
>> >> >> >> apic: apic_set_irq: coalescing 0
>> >> >> >>
>> >> > So interrupt is _alway_ coalesced. If apic_get_irq_delivered() returns
>> >> > 0 it means the interrupt was not delivered.
>> >>
>> >> That seems strange. I changed the program so that the handler gets
>> >> executed, also output a dot to serial from the handler. I changed the
>> >> frequency to 2Hz.
>> >>
>> >> Now, if I leave out -rtc-td-hack, I get the dots at 2Hz as expected.
>> >> With -rtc-td-hack, the dots come out much faster. I added
>> >> DEBUG_COALESCING also to RTC, with that enabled I get:
>> >> qemu -L . -bios coalescing.bin -d in_asm,int -no-hpet -rtc-td-hack 
>> >> -serial stdio
>> >> cmos: coalesced irqs scaled to 0
>> >> cmos: coalesced irqs increased to 1
>> >> cmos: injecting on ack
>> >> .cmos: injecting on ack
>> >> .cmos: injecting on ack
>> >> .cmos: injecting on ack
>> >> .cmos: injecting on ack
>> >> .cmos: injecting on ack
>> >> .cmos: injecting on ack
>> >> .cmos: injecting on ack
>> >> .cmos: injecting on ack
>> >> .cmos: injecting on ack
>> >> .cmos: injecting on ack
>> >> .cmos: injecting on ack
>> >> .cmos: injecting on ack
>> >> .cmos: injecting on ack
>> >> .cmos: injecting on ack
>> >> .cmos: injecting on ack
>> >> .cmos: injecting on ack
>> >> .cmos: injecting on ack
>> >> .cmos: injecting on ack
>> >> .cmos: injecting on ack
>> >> ..cmos: injecting from timer
>> >> .cmos: coalesced irqs increased to 2
>> >> cmos: injecting on ack
>> >>
>> >> So, there are bogus injections.
>> >
>> > Looks like irr in apic is never cleared. Probably bug in userspace apic
>> > emulation. I'll look into it. Try to route interrupt via APIC (not ExtInt),
>> > or use qemu-kvm with in kernel irq chip.
>>
>> With APIC you mean Fixed? Then the IRQ is not delivered at all.
> You need to deliver it through IOAPIC.

In this version, when USE_APIC is defined, IRQs are routed via IOAPIC
and APIC, PIC interrupts are disabled. Otherwise, IOAPIC and APIC are
left to reset state and PIC is used.

With the PIC version, there are bogus injections. With IOAPIC and APIC
dots come at 2Hz. -enable-kvm has no effect.


coalescing.S
Description: Binary data


coalescing_apic.bin.bz2
Description: BZip2 compressed data


coalescing_pic.bin.bz2
Description: BZip2 compressed data


[Qemu-devel] [PATCH] x86: svm: Always clear event_inj on vmexit

2010-06-01 Thread Jan Kiszka
We currently only clear SVM_EVTINJ_VALID after successful interrupt
delivery. This apparently does not match real hardware which clears the
whole event_inj field on every vmexit, including unsuccessful interrupt
delivery.

Reported-by: Erik van der Kouwe 
Signed-off-by: Jan Kiszka 
---

(before it gets lost)
Erik, please confirm that this works for you.

 target-i386/op_helper.c |8 +---
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
index dcbdfe7..caabdb4 100644
--- a/target-i386/op_helper.c
+++ b/target-i386/op_helper.c
@@ -1263,13 +1263,6 @@ void do_interrupt(int intno, int is_int, int error_code,
 #endif
 do_interrupt_real(intno, is_int, error_code, next_eip);
 }
-
-#if !defined(CONFIG_USER_ONLY)
-if (env->hflags & HF_SVMI_MASK) {
-   uint32_t event_inj = ldl_phys(env->vm_vmcb + offsetof(struct vmcb, 
control.event_inj));
-   stl_phys(env->vm_vmcb + offsetof(struct vmcb, control.event_inj), 
event_inj & ~SVM_EVTINJ_VALID);
-}
-#endif
 }
 
 /* This should come from sysemu.h - if we could include it here... */
@@ -5388,6 +5381,7 @@ void helper_vmexit(uint32_t exit_code, uint64_t 
exit_info_1)
  ldl_phys(env->vm_vmcb + offsetof(struct vmcb, 
control.event_inj)));
 stl_phys(env->vm_vmcb + offsetof(struct vmcb, control.exit_int_info_err),
  ldl_phys(env->vm_vmcb + offsetof(struct vmcb, 
control.event_inj_err)));
+stl_phys(env->vm_vmcb + offsetof(struct vmcb, control.event_inj), 0);
 
 env->hflags2 &= ~HF2_GIF_MASK;
 /* FIXME: Resets the current ASID register to zero (host ASID). */
-- 
1.6.0.2



[Qemu-devel] Re: [PATCH] sparc32 esp fix spurious interrupts in chip reset

2010-06-01 Thread Blue Swirl
On Sun, May 30, 2010 at 10:35 PM, Artyom Tarasenko
 wrote:
> lower interrupt during chip reset. Otherwise the ESP_RSTAT register
> may get out of sync with the IRQ line status. This effect became
> visible after commit 65899fe3

Hard reset handlers should not touch qemu_irqs, because on cold start,
the receiving end may be unprepared to handle the signal. See
0d0a7e69e853639b123798877e019c3c7ee6634a,
bc26e55a6615dc594be425d293db40d5cdcdb84b and
42f1ced228c9b616cfa2b69846025271618e4ef5.

For ESP there are two other sources of reset: signal from DMA and chip
reset command. On those cases, lowering IRQ makes sense.

So the correct fix is to refactor the reset handling a bit. Does this
patch also fix your test case?
From 44d40083652d5135ecb567fea4cff4f109ee4cd0 Mon Sep 17 00:00:00 2001
From: Blue Swirl 
Date: Tue, 1 Jun 2010 17:37:13 +
Subject: [PATCH] esp: lower IRQ on soft reset

42f1ced228c9b616cfa2b69846025271618e4ef5 removed irq lowering
during reset. However, for chip reset command and DMA reset signal,
its actually the correct thing to do.

Lower IRQ on soft reset only.

Signed-off-by: Blue Swirl 
---
 hw/esp.c |   19 ++-
 1 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/hw/esp.c b/hw/esp.c
index 0a8cf6e..7740879 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -419,7 +419,7 @@ static void handle_ti(ESPState *s)
 }
 }
 
-static void esp_reset(DeviceState *d)
+static void esp_hard_reset(DeviceState *d)
 {
 ESPState *s = container_of(d, ESPState, busdev.qdev);
 
@@ -435,10 +435,19 @@ static void esp_reset(DeviceState *d)
 s->rregs[ESP_CFG1] = 7;
 }
 
+static void esp_soft_reset(DeviceState *d)
+{
+ESPState *s = container_of(d, ESPState, busdev.qdev);
+
+qemu_irq_lower(s->irq);
+esp_hard_reset(d);
+}
+
 static void parent_esp_reset(void *opaque, int irq, int level)
 {
-if (level)
-esp_reset(opaque);
+if (level) {
+esp_soft_reset(opaque);
+}
 }
 
 static uint32_t esp_mem_readb(void *opaque, target_phys_addr_t addr)
@@ -528,7 +537,7 @@ static void esp_mem_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
 break;
 case CMD_RESET:
 DPRINTF("Chip reset (%2.2x)\n", val);
-esp_reset(&s->busdev.qdev);
+esp_soft_reset(&s->busdev.qdev);
 break;
 case CMD_BUSRESET:
 DPRINTF("Bus reset (%2.2x)\n", val);
@@ -679,7 +688,7 @@ static SysBusDeviceInfo esp_info = {
 .qdev.name  = "esp",
 .qdev.size  = sizeof(ESPState),
 .qdev.vmsd  = &vmstate_esp,
-.qdev.reset = esp_reset,
+.qdev.reset = esp_hard_reset,
 .qdev.props = (Property[]) {
 {.name = NULL}
 }
-- 
1.5.6.5



[Qemu-devel] Re: [PATCH 2/3] QMP: Introduce commands documentation

2010-06-01 Thread Jan Kiszka
Luiz Capitulino wrote:
> On Tue, 01 Jun 2010 18:10:26 +0200
> Jan Kiszka  wrote:
> 
>> Luiz Capitulino wrote:
>>> From: Jan Kiszka 
>>>
>>> One of the most important missing feature in QMP today is its
>>> supported commands documentation.
>>>
>>> The plan is to make it part of self-description support, however
>>> self-description is a big task we have been postponing for a
>>> long time now and still don't know when it's going to be done.
>>>
>>> In order not to compromise QMP adoption and make users' life easier,
>>> this commit adds a simple text documentation which fully describes
>>> all QMP supported commands.
>>>
>>> This is not ideal for a number of reasons (harder to maintain,
>>> text-only, etc) but does improve the current situation. To avoid at
>>> least divering from the user monitor help and texi snippets, QMP bits
>>> are also maintained inside qemu-monitor.hx, and hxtool is extended to
>>> generate a single text file from them.
>>>
>>> Signed-off-by: Jan Kiszka 
>>> Signed-off-by: Luiz Capitulino 
>>> ---
>>>  Makefile|5 +-
>>>  QMP/README  |5 +-
>>>  configure   |4 +
>>>  hxtool  |   44 ++-
>>>  qemu-monitor.hx | 1322 
>>> +++
>>>  5 files changed, 1376 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 7986bf6..3a8a311 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -29,7 +29,7 @@ $(call set-vpath, $(SRC_PATH):$(SRC_PATH)/hw)
>>>  LIBS+=-lz $(LIBS_TOOLS)
>>>
>>>  ifdef BUILD_DOCS
>>> -DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8
>>> +DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8 
>>> QMP/qmp-commands.txt
>>>  else
>>>  DOCS=
>>>  endif
>>> @@ -259,6 +259,9 @@ qemu-options.texi: $(SRC_PATH)/qemu-options.hx
>>>  qemu-monitor.texi: $(SRC_PATH)/qemu-monitor.hx
>>> $(call quiet-command,sh $(SRC_PATH)/hxtool -t < $< > $@,"  GEN   
>>> $@")
>>>
>>> +QMP/qmp-commands.txt: $(SRC_PATH)/qemu-monitor.hx
>>> +   $(call quiet-command,sh $(SRC_PATH)/hxtool -q < $< > $@,"  GEN   
>>> $@")
>>> +
>>>  qemu-img-cmds.texi: $(SRC_PATH)/qemu-img-cmds.hx
>>> $(call quiet-command,sh $(SRC_PATH)/hxtool -t < $< > $@,"  GEN   
>>> $@")
>>>
>> This hunk seem to have "gained" tab-to-spaces conversion, unfortunately
>> in a makefile.
>>
>> Applying the patch generated another hunk warning:
> 
>  Are those problems worth a resping or can we fix then with
> additional patches? It works ok here.

Oh, it's indeed a local issue. Likely our corporate mail server. Nice...
Sorry for the noise!

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH -V4 7/7] virtio-9p: Implemented security model for chown and chgrp.

2010-06-01 Thread Aneesh Kumar K.V
On Wed, May 26, 2010 at 04:21:46PM -0700, Venkateswararao Jujjuri (JV) wrote:
> Signed-off-by: Venkateswararao Jujjuri 
> ---
>  hw/file-op-9p.h  |4 ++--
>  hw/virtio-9p-local.c |   18 ++
>  hw/virtio-9p.c   |   15 ---
>  3 files changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/file-op-9p.h b/hw/file-op-9p.h
> index c1c08b4..877faf2 100644
> --- a/hw/file-op-9p.h
> +++ b/hw/file-op-9p.h
> @@ -49,8 +49,8 @@ typedef struct FileOperations
>  {
>  int (*lstat)(FsContext *, const char *, struct stat *);
>  ssize_t (*readlink)(FsContext *, const char *, char *, size_t);
> -int (*chmod)(FsContext *, const char *, mode_t);
> -int (*chown)(FsContext *, const char *, uid_t, gid_t);
> +int (*chmod)(FsContext *, const char *, FsCred *);
> +int (*chown)(FsContext *, const char *, FsCred *);
>  int (*mknod)(FsContext *, const char *, FsCred *);
>  int (*utime)(FsContext *, const char *, const struct utimbuf *);
>  int (*remove)(FsContext *, const char *);
> diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
> index 11f3650..f46acac 100644
> --- a/hw/virtio-9p-local.c
> +++ b/hw/virtio-9p-local.c
> @@ -173,9 +173,14 @@ static ssize_t local_writev(FsContext *ctx, int fd, 
> const struct iovec *iov,
>  return writev(fd, iov, iovcnt);
>  }
>  
> -static int local_chmod(FsContext *ctx, const char *path, mode_t mode)
> +static int local_chmod(FsContext *fs_ctx, const char *path, FsCred *credp)
>  {
> -return chmod(rpath(ctx, path), mode);
> +if (fs_ctx->fs_sm == SM_MAPPED) {
> +return local_set_xattr(rpath(fs_ctx, path), credp);
> +} else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
> +return chmod(rpath(fs_ctx, path), credp->fc_mode);
> +}
> +return -1;
>  }

You should use this in open2.



>  
>  static int local_mknod(FsContext *fs_ctx, const char *path, FsCred *credp)
> @@ -436,9 +441,14 @@ static int local_rename(FsContext *ctx, const char 
> *oldpath,
>  
>  }
>  
> -static int local_chown(FsContext *ctx, const char *path, uid_t uid, gid_t 
> gid)
> +static int local_chown(FsContext *fs_ctx, const char *path, FsCred *credp)
>  {
> -return chown(rpath(ctx, path), uid, gid);
> +if (fs_ctx->fs_sm == SM_MAPPED) {
> +return local_set_xattr(rpath(fs_ctx, path), credp);
> +} else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
> +return chown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid);
> +}
> +return -1;
>  }
>  

Same here. So that we don't have 

if (fs_ctx->fs_sm == SM_MAPPED) spread in the open2 code but is logically 
grouped at the right place.


>  static int local_utime(FsContext *ctx, const char *path,
> diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
> index 90620aa..dceb5fc 100644
> --- a/hw/virtio-9p.c
> +++ b/hw/virtio-9p.c
> @@ -154,7 +154,11 @@ static int v9fs_do_writev(V9fsState *s, int fd, const 
> struct iovec *iov,
>  
>  static int v9fs_do_chmod(V9fsState *s, V9fsString *path, mode_t mode)
>  {
> -return s->ops->chmod(&s->ctx, path->data, mode);
> +FsCred cred;
> +cred_init(&cred);
> +cred.fc_mode = mode;
> +
> +return s->ops->chmod(&s->ctx, path->data, &cred);
>  }
>  
>  static int v9fs_do_mknod(V9fsState *s, V9fsCreateState *vs, mode_t mode,
> @@ -231,7 +235,12 @@ static int v9fs_do_rename(V9fsState *s, V9fsString 
> *oldpath,
>  
>  static int v9fs_do_chown(V9fsState *s, V9fsString *path, uid_t uid, gid_t 
> gid)
>  {
> -return s->ops->chown(&s->ctx, path->data, uid, gid);
> +FsCred cred;
> +cred_init(&cred);
> +cred.fc_uid = uid;
> +cred.fc_gid = gid;
> +
> +return s->ops->chown(&s->ctx, path->data, &cred);
>  }
>  
>  static int v9fs_do_utime(V9fsState *s, V9fsString *path,
> @@ -2022,7 +2031,7 @@ static void v9fs_wstat_post_utime(V9fsState *s, 
> V9fsWstatState *vs, int err)
>  goto out;
>  }
>  
> -if (vs->v9stat.n_gid != -1) {
> +if (vs->v9stat.n_gid != -1 || vs->v9stat.n_uid != -1) {
>  if (v9fs_do_chown(s, &vs->fidp->path, vs->v9stat.n_uid,
>  vs->v9stat.n_gid)) {
>  err = -errno;
> -- 
> 1.6.5.2
> 
> 

-aneesh



Re: [Qemu-devel] [PATCH -V4 3/7] virtio-9p: modify create/open2 and mkdir for new security model.

2010-06-01 Thread Aneesh Kumar K.V
On Wed, May 26, 2010 at 04:21:42PM -0700, Venkateswararao Jujjuri (JV) wrote:
> Add required infrastructure and modify create/open2 and mkdir per the new
> security model.
> 
> Signed-off-by: Venkateswararao Jujjuri 
> ---
>  hw/file-op-9p.h  |   23 +++-
>  hw/virtio-9p-local.c |  149 
> --
>  hw/virtio-9p.c   |   42 ++
>  3 files changed, 158 insertions(+), 56 deletions(-)
> 
> diff --git a/hw/file-op-9p.h b/hw/file-op-9p.h
> index 2934ff1..73d59b2 100644
> --- a/hw/file-op-9p.h
> +++ b/hw/file-op-9p.h
> @@ -19,13 +19,32 @@
>  #include 
>  #include 
>  #include 
> +#define SM_LOCAL_MODE_BITS0600
> +#define SM_LOCAL_DIR_MODE_BITS0700
> +
> +typedef enum
> +{
> +SM_PASSTHROUGH = 1, /* uid/gid set on fileserver files */
> +SM_MAPPED,  /* uid/gid part of xattr */
> +} SecModel;
> +
> +typedef struct FsCred
> +{
> +uid_t   fc_uid;
> +gid_t   fc_gid;
> +mode_t  fc_mode;
> +dev_t   fc_rdev;
> +} FsCred;
>  
>  typedef struct FsContext
>  {
>  char *fs_root;
> +SecModel fs_sm;
>  uid_t uid;
>  } FsContext;
>  
> +extern void cred_init(FsCred *);
> +
>  typedef struct FileOperations
>  {
>  int (*lstat)(FsContext *, const char *, struct stat *);
> @@ -43,7 +62,7 @@ typedef struct FileOperations
>  int (*closedir)(FsContext *, DIR *);
>  DIR *(*opendir)(FsContext *, const char *);
>  int (*open)(FsContext *, const char *, int);
> -int (*open2)(FsContext *, const char *, int, mode_t);
> +int (*open2)(FsContext *, const char *, int, FsCred *);
>  void (*rewinddir)(FsContext *, DIR *);
>  off_t (*telldir)(FsContext *, DIR *);
>  struct dirent *(*readdir)(FsContext *, DIR *);
> @@ -51,7 +70,7 @@ typedef struct FileOperations
>  ssize_t (*readv)(FsContext *, int, const struct iovec *, int);
>  ssize_t (*writev)(FsContext *, int, const struct iovec *, int);
>  off_t (*lseek)(FsContext *, int, off_t, int);
> -int (*mkdir)(FsContext *, const char *, mode_t);
> +int (*mkdir)(FsContext *, const char *, FsCred *);
>  int (*fstat)(FsContext *, int, struct stat *);
>  int (*rename)(FsContext *, const char *, const char *);
>  int (*truncate)(FsContext *, const char *, off_t);
> diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
> index 78960ac..f6c2fe2 100644
> --- a/hw/virtio-9p-local.c
> +++ b/hw/virtio-9p-local.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  static const char *rpath(FsContext *ctx, const char *path)
>  {
> @@ -31,47 +32,39 @@ static int local_lstat(FsContext *ctx, const char *path, 
> struct stat *stbuf)
>  return lstat(rpath(ctx, path), stbuf);
>  }
>  
> -static int local_setuid(FsContext *ctx, uid_t uid)
> +static int local_set_xattr(const char *path, FsCred *credp)
>  {
> -struct passwd *pw;
> -gid_t groups[33];
> -int ngroups;
> -static uid_t cur_uid = -1;
> -
> -if (cur_uid == uid) {
> -return 0;
> -}
> -
> -if (setreuid(0, 0)) {
> -return -1;
> -}
> -
> -pw = getpwuid(uid);
> -if (pw == NULL) {
> -return -1;
> +int err;
> +if (credp->fc_uid != -1) {
> +err = setxattr(path, "user.virtfs.uid", &credp->fc_uid, 
> sizeof(uid_t),
> +0);
> +if (err) {
> +return err;
> +}
>  }
> -
> -ngroups = 33;
> -if (getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups) == -1) {
> -return -1;
> +if (credp->fc_gid != -1) {
> +err = setxattr(path, "user.virtfs.gid", &credp->fc_gid, 
> sizeof(gid_t),
> +0);
> +if (err) {
> +return err;
> +}
>  }
> -
> -if (setgroups(ngroups, groups)) {
> -return -1;
> -}
> -
> -if (setregid(-1, pw->pw_gid)) {
> -return -1;
> +if (credp->fc_mode != -1) {
> +err = setxattr(path, "user.virtfs.mode", &credp->fc_mode,
> +sizeof(mode_t), 0);
> +if (err) {
> +return err;
> +}
>  }
> -
> -if (setreuid(-1, uid)) {
> -return -1;
> +if (credp->fc_rdev != -1) {
> +err = setxattr(path, "user.virtfs.rdev", &credp->fc_rdev,
> +sizeof(dev_t), 0);
> +if (err) {
> +return err;
> +}
>  }
> -
> -cur_uid = uid;
> -
> -return 0;
> -}
> + return 0;
> + }
>  
>  static ssize_t local_readlink(FsContext *ctx, const char *path,
>  char *buf, size_t bufsz)
> @@ -168,9 +161,44 @@ static int local_mksock(FsContext *ctx2, const char 
> *path)
>  return 0;
>  }
>  
> -static int local_mkdir(FsContext *ctx, const char *path, mode_t mode)
> +static int local_mkdir(FsContext *fs_ctx, const char *path, FsCred *credp)
>  {
> -return mkdir(rpath(ctx, path), mode);
> +int err = -1;
> +int serrno = 0;
> +/* Determine the security model */
> +if (fs_ctx->fs_sm == SM_MAPPED) {
> +   

[Qemu-devel] Re: [PATCH 2/3] QMP: Introduce commands documentation

2010-06-01 Thread Luiz Capitulino
On Tue, 01 Jun 2010 18:10:26 +0200
Jan Kiszka  wrote:

> Luiz Capitulino wrote:
> > From: Jan Kiszka 
> > 
> > One of the most important missing feature in QMP today is its
> > supported commands documentation.
> > 
> > The plan is to make it part of self-description support, however
> > self-description is a big task we have been postponing for a
> > long time now and still don't know when it's going to be done.
> > 
> > In order not to compromise QMP adoption and make users' life easier,
> > this commit adds a simple text documentation which fully describes
> > all QMP supported commands.
> > 
> > This is not ideal for a number of reasons (harder to maintain,
> > text-only, etc) but does improve the current situation. To avoid at
> > least divering from the user monitor help and texi snippets, QMP bits
> > are also maintained inside qemu-monitor.hx, and hxtool is extended to
> > generate a single text file from them.
> > 
> > Signed-off-by: Jan Kiszka 
> > Signed-off-by: Luiz Capitulino 
> > ---
> >  Makefile|5 +-
> >  QMP/README  |5 +-
> >  configure   |4 +
> >  hxtool  |   44 ++-
> >  qemu-monitor.hx | 1322 
> > +++
> >  5 files changed, 1376 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index 7986bf6..3a8a311 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -29,7 +29,7 @@ $(call set-vpath, $(SRC_PATH):$(SRC_PATH)/hw)
> >  LIBS+=-lz $(LIBS_TOOLS)
> > 
> >  ifdef BUILD_DOCS
> > -DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8
> > +DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8 
> > QMP/qmp-commands.txt
> >  else
> >  DOCS=
> >  endif
> > @@ -259,6 +259,9 @@ qemu-options.texi: $(SRC_PATH)/qemu-options.hx
> >  qemu-monitor.texi: $(SRC_PATH)/qemu-monitor.hx
> > $(call quiet-command,sh $(SRC_PATH)/hxtool -t < $< > $@,"  GEN   
> > $@")
> > 
> > +QMP/qmp-commands.txt: $(SRC_PATH)/qemu-monitor.hx
> > +   $(call quiet-command,sh $(SRC_PATH)/hxtool -q < $< > $@,"  GEN   
> > $@")
> > +
> >  qemu-img-cmds.texi: $(SRC_PATH)/qemu-img-cmds.hx
> > $(call quiet-command,sh $(SRC_PATH)/hxtool -t < $< > $@,"  GEN   
> > $@")
> > 
> 
> This hunk seem to have "gained" tab-to-spaces conversion, unfortunately
> in a makefile.
> 
> Applying the patch generated another hunk warning:

 Are those problems worth a resping or can we fix then with
additional patches? It works ok here.



Re: [Qemu-devel] [PATCH -V4 1/7] virtio-9p: Introduces an option to specify the security model.

2010-06-01 Thread Aneesh Kumar K.V
On Wed, May 26, 2010 at 04:21:40PM -0700, Venkateswararao Jujjuri (JV) wrote:
> The new option is:
> 
> -fsdev fstype,id=myid,path=/share_path/,security_model=[mapped|passthrough]
> -virtfs 
> fstype,path=/share_path/,security_model=[mapped|passthrough],mnt_tag=tag
> 
> In the case of mapped security model, files are created with QEMU user
> credentials and the client-user's credentials are saved in extended 
> attributes.
> Whereas in the case of passthrough security model, files on the
> filesystem are directly created with client-user's credentials.
> 
> Signed-off-by: Venkateswararao Jujjuri 
> ---
>  fsdev/qemu-fsdev.c |   14 +-
>  fsdev/qemu-fsdev.h |1 +
>  hw/virtio-9p.c |   22 ++
>  qemu-config.c  |   12 +---
>  qemu-options.hx|   15 +++
>  vl.c   |   18 +++---
>  6 files changed, 67 insertions(+), 15 deletions(-)
> 
> diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
> index 813e1f7..7d7a153 100644
> --- a/fsdev/qemu-fsdev.c
> +++ b/fsdev/qemu-fsdev.c
> @@ -34,7 +34,7 @@ int qemu_fsdev_add(QemuOpts *opts)
>  return -1;
>  }
>  
> - for (i = 0; i < ARRAY_SIZE(FsTypes); i++) {
> +for (i = 0; i < ARRAY_SIZE(FsTypes); i++) {
>  if (strcmp(FsTypes[i].name, qemu_opt_get(opts, "fstype")) == 0) {
>  break;
>  }


Don't do whitespace fixup in the same patch.


> @@ -46,10 +46,22 @@ int qemu_fsdev_add(QemuOpts *opts)
>  return -1;
>  }
>  
> +if (qemu_opt_get(opts, "path") == NULL) {
> +fprintf(stderr, "fsdev: No path specified.\n");
> +return -1;
> +}
> +


How is this related to new option ? 


> +if (qemu_opt_get(opts, "security_model") == NULL) {
> +fprintf(stderr, "fsdev: No security_model specified.\n");
> +return -1;
> +}
> +
>  fsle = qemu_malloc(sizeof(*fsle));
>  
>  fsle->fse.fsdev_id = qemu_strdup(qemu_opts_id(opts));
>  fsle->fse.path = qemu_strdup(qemu_opt_get(opts, "path"));
> +fsle->fse.security_model = qemu_strdup(qemu_opt_get(opts,
> +"security_model"));
>  fsle->fse.ops = FsTypes[i].ops;
>  
>  QTAILQ_INSERT_TAIL(&fstype_entries, fsle, next);
> diff --git a/fsdev/qemu-fsdev.h b/fsdev/qemu-fsdev.h
> index b50fbe0..6c27881 100644
> --- a/fsdev/qemu-fsdev.h
> +++ b/fsdev/qemu-fsdev.h
> @@ -40,6 +40,7 @@ typedef struct FsTypeTable {
>  typedef struct FsTypeEntry {
>  char *fsdev_id;
>  char *path;
> +char *security_model;
>  FileOperations *ops;
>  } FsTypeEntry;
>  
> diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
> index 687abc0..a57562a 100644
> --- a/hw/virtio-9p.c
> +++ b/hw/virtio-9p.c
> @@ -2402,7 +2402,7 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf 
> *conf)
>  /* We don't have a fsdev identified by fsdev_id */
>  fprintf(stderr, "Virtio-9p device couldn't find fsdev "
>  "with the id %s\n", conf->fsdev_id);
> -exit(1);
> +return NULL;
>  }
>  
>  if (!fse->path || !conf->tag) {
> @@ -2410,15 +2410,29 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, 
> V9fsConf *conf)
>  fprintf(stderr, "fsdev with id %s needs path "
>  "and Virtio-9p device needs mount_tag arguments\n",
>  conf->fsdev_id);
> -exit(1);
> +return NULL;
> +}
> +
> +if (!strcmp(fse->security_model, "passthrough")) {
> +/* Files on the Fileserver set to client user credentials */
> +} else if (!strcmp(fse->security_model, "mapped")) {
> +/* Files on the fileserver are set to QEMU credentials.
> + * Client user credentials are saved in extended attributes.
> + */


The above two if should be dropped add them when you have something to do in 
the if () { }
section.


> +} else {
> +/* user haven't specified a correct security option */
> +fprintf(stderr, "one of the following must be specified as the"
> +"security option:\n\t security_model=passthrough \n\t "
> +"security_model=mapped\n");
> +return NULL;
>  }

We should only have this



>  
>  if (lstat(fse->path, &stat)) {
>  fprintf(stderr, "share path %s does not exist\n", fse->path);
> -exit(1);
> +return NULL;
>  } else if (!S_ISDIR(stat.st_mode)) {
>  fprintf(stderr, "share path %s is not a directory \n", fse->path);
> -exit(1);
> +return NULL;
>  }
>  
>  s->ctx.fs_root = qemu_strdup(fse->path);
> diff --git a/qemu-config.c b/qemu-config.c
> index d500885..e1e3aa1 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -160,9 +160,12 @@ QemuOptsList qemu_fsdev_opts = {
>  {
>  .name = "fstype",
>  .type = QEMU_OPT_STRING,
> -}, {
> +},{
>  .name = "path",
>  .type = QEMU_OPT_STRING,
> +},{
> +.name = "security_model",
> +

Re: [Qemu-devel] [PATCH -V4 2/7] virtio-9p: Rearrange fileop structures

2010-06-01 Thread Aneesh Kumar K.V
On Wed, May 26, 2010 at 04:21:41PM -0700, Venkateswararao Jujjuri (JV) wrote:
> Signed-off-by: Venkateswararao Jujjuri 
> ---
>  hw/virtio-9p.c |  185 
> ++--
>  hw/virtio-9p.h |   92 
>  2 files changed, 138 insertions(+), 139 deletions(-)

This is a cleanup patch. I guess this should not be part of this series.

-aneesh




Re: [Qemu-devel] [PATCH] block: Assume raw for drives without media

2010-06-01 Thread Nicholas A. Bellinger
On Tue, 2010-06-01 at 18:50 +0200, Kevin Wolf wrote:
> qemu -cdrom /dev/cdrom with an empty CD-ROM drive doesn't work any more 
> because
> we try to guess the format and when this fails (because there is no medium) we
> exit with an error message.
> 
> This patch should restore the old behaviour by assuming raw format for such
> drives.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block.c |4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/block.c b/block.c
> index d789d02..7dded4e 100644
> --- a/block.c
> +++ b/block.c
> @@ -331,8 +331,8 @@ static BlockDriver *find_image_format(const char 
> *filename)
>  if (ret < 0)
>  return NULL;
>  
> -/* Return the raw BlockDriver * to scsi-generic devices */
> -if (bs->sg) {
> +/* Return the raw BlockDriver * to scsi-generic devices or empty drives 
> */
> +if (bs->sg || !bdrv_is_inserted(bs)) {
>  bdrv_delete(bs);
>  return bdrv_find_format("raw");
>  }

Makes sense to me.

Acked-by: Nicholas A. Bellinger 




Re: [Qemu-devel] Re: [PULL] pci, vhost fixes

2010-06-01 Thread Blue Swirl
On Tue, Jun 1, 2010 at 5:32 AM, Markus Armbruster  wrote:
> Blue Swirl  writes:
>
>> On Mon, May 31, 2010 at 1:35 PM, Michael S. Tsirkin  wrote:
>>> On Mon, May 31, 2010 at 02:51:26PM +0200, Paolo Bonzini wrote:
 On 05/30/2010 08:19 PM, Blue Swirl wrote:
> We have PRI*64 just for this purpose, so let's use them. The
> discussion about that was earlier.

 How does mingw matter for vhost-net specific code?

 Paolo
>>>
>>> Good point. That code is linux-only, so we are safe using %lld.
>>> By the time someone ports vhost to windows, there's a chance
>>> debian will update it's mingw.  Blue Swirl?
>>
>> It's a good point, but there's also consistency: now no '%ll*' formats are 
>> used.
>
> If it ain't broke, don't fix it.  Especially not when the fix is ugly.

Fully agree. But because beauty is in the eye of the beholder, our
conclusions are probably different. ;-)



[Qemu-devel] [PATCH] block: Assume raw for drives without media

2010-06-01 Thread Kevin Wolf
qemu -cdrom /dev/cdrom with an empty CD-ROM drive doesn't work any more because
we try to guess the format and when this fails (because there is no medium) we
exit with an error message.

This patch should restore the old behaviour by assuming raw format for such
drives.

Signed-off-by: Kevin Wolf 
---
 block.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index d789d02..7dded4e 100644
--- a/block.c
+++ b/block.c
@@ -331,8 +331,8 @@ static BlockDriver *find_image_format(const char *filename)
 if (ret < 0)
 return NULL;
 
-/* Return the raw BlockDriver * to scsi-generic devices */
-if (bs->sg) {
+/* Return the raw BlockDriver * to scsi-generic devices or empty drives */
+if (bs->sg || !bdrv_is_inserted(bs)) {
 bdrv_delete(bs);
 return bdrv_find_format("raw");
 }
-- 
1.6.6.1




Re: [Qemu-devel] Re: [PATCH 2/4] migration-tcp: threaded tcp incoming migration.

2010-06-01 Thread Yoshiaki Tamura
2010/6/2 Anthony Liguori :
> On 06/01/2010 11:23 AM, Yoshiaki Tamura wrote:
>>
>> 2010/6/2 Anthony Liguori:
>>
>>>
>>> On 06/01/2010 10:40 AM, Yoshiaki Tamura wrote:
>>>

 Create a thread to handle tcp incoming migration when CONFIG_IOTHREAD
 is enabled.  Spawned thread writes it's return status to th_fds[1]
 before exit, and main thread joins and reads it.  In
 tcp_start_incoming_migration(), allocate FdMigrationState and return
 MigrationState to let migration to print incoming migration info.


>>>
>>> In the absence of any locking, I can't see how this is safe.
>>>
>>
>> Right.  If we use threading here, we need to prevent commands from
>> monitor that affects incoming thread.
>>
>
> There's a huge number of calls that can get made during live migration that
> can also be triggered by the monitor.
>
> Even the basic things likes qemu_set_fd_handler2 can cause havoc.
>
> Just preventing certain monitor commands is not sufficient.

Ah, that was my concern.  Sorry for not understanding well.
Thanks for your advice.

Yoshi

>
> Regards,
>
> Anthony Liguori
>
>> Thanks,
>>
>> Yoshi
>>
>>
>>>
>>> Regards,
>>>
>>> Anthony Liguori
>>>
>>>

 Signed-off-by: Yoshiaki Tamura
 ---
  migration-tcp.c |   86
 ++-
  migration.h     |    2 +-
  2 files changed, 73 insertions(+), 15 deletions(-)

 diff --git a/migration-tcp.c b/migration-tcp.c
 index 95ce722..f20e5fe 100644
 --- a/migration-tcp.c
 +++ b/migration-tcp.c
 @@ -18,6 +18,7 @@
  #include "sysemu.h"
  #include "buffered_file.h"
  #include "block.h"
 +#include "qemu-thread.h"

  //#define DEBUG_MIGRATION_TCP

 @@ -29,6 +30,11 @@
      do { } while (0)
  #endif

 +#ifdef CONFIG_IOTHREAD
 +static QemuThread migration_thread;
 +static int th_fds[2];
 +#endif
 +
  static int socket_errno(FdMigrationState *s)
  {
      return socket_error();
 @@ -176,41 +182,93 @@ static void tcp_accept_incoming_migration(void
 *opaque)
  out_fopen:
      qemu_fclose(f);
  out:
 +#ifndef CONFIG_IOTHREAD
      qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
 +#endif
      close(s);
      close(c);
 +#ifdef CONFIG_IOTHREAD
 +    write(th_fds[1],&ret, sizeof(ret));
 +    qemu_thread_exit(NULL);
 +#endif
 +}
 +
 +#ifdef CONFIG_IOTHREAD
 +static void tcp_incoming_migration_complete(void *opaque)
 +{
 +    int ret, state = 0;
 +    FdMigrationState *s = opaque;
 +
 +    qemu_thread_join(&migration_thread, NULL);
 +
 +    ret = read(th_fds[0],&state, sizeof(state));
 +    if (ret == -1) {
 +        fprintf(stderr, "failed to read from pipe\n");
 +        goto err;
 +    }
 +
 +    s->state = state<    0 ? MIG_STATE_ERROR : MIG_STATE_COMPLETED;
 +
 +err:
 +    qemu_set_fd_handler2(th_fds[0], NULL, NULL, NULL, NULL);
 +    close(th_fds[0]);
 +    close(th_fds[1]);
  }
 +#endif

 -int tcp_start_incoming_migration(const char *host_port)
 +MigrationState *tcp_start_incoming_migration(const char *host_port)
  {
      struct sockaddr_in addr;
 +    FdMigrationState *s;
      int val;
 -    int s;

      if (parse_host_port(&addr, host_port)<    0) {
          fprintf(stderr, "invalid host/port combination: %s\n",
 host_port);
 -        return -EINVAL;
 +        return NULL;
      }

 -    s = qemu_socket(PF_INET, SOCK_STREAM, 0);
 -    if (s == -1)
 -        return -socket_error();
 +    s = qemu_mallocz(sizeof(*s));
 +
 +    s->get_error = socket_errno;
 +    s->close = tcp_close;
 +    s->mig_state.cancel = migrate_fd_cancel;
 +    s->mig_state.get_status = migrate_fd_get_status;
 +    s->state = MIG_STATE_ACTIVE;
 +
 +    s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
 +    if (s->fd == -1) {
 +        qemu_free(s);
 +        return NULL;
 +    }

      val = 1;
 -    setsockopt(s, SOL_SOCKET, SO_REUSEADDR, (const char *)&val,
 sizeof(val));
 +    setsockopt(s->fd, SOL_SOCKET, SO_REUSEADDR,
 +               (const char *)&val, sizeof(val));

 -    if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) == -1)
 +    if (bind(s->fd, (struct sockaddr *)&addr, sizeof(addr)) == -1)
          goto err;

 -    if (listen(s, 1) == -1)
 +    if (listen(s->fd, 1) == -1)
          goto err;

 -    qemu_set_fd_handler2(s, NULL, tcp_accept_incoming_migration, NULL,
 -                         (void *)(unsigned long)s);
 +#ifdef CONFIG_IOTHREAD
 +    if (qemu_pipe(th_fds) == -1) {
 +        fprintf(stderr, "failed to create pipe\n");
 +        goto err;
 +    }

 -    return 0;
 +    qemu_thread_create(&migration_thread, (void
 

Re: [Qemu-devel] Re: [PATCH 0/4] Threaded tcp incoming migration.

2010-06-01 Thread Yoshiaki Tamura
2010/6/2 Anthony Liguori :
> On 06/01/2010 11:18 AM, Yoshiaki Tamura wrote:
>>
>> 2010/6/2 Anthony Liguori:
>>
>>>
>>> On 06/01/2010 10:40 AM, Yoshiaki Tamura wrote:
>>>

 Hi,

 This series add threaded tcp incoming migration.  Currently, tcp
 migration
 on
 incoming side is blocked when outgoing has started on the remote side,
 and
 you
 can't do anything.  With this series you can get info of incoming
 migration by
 calling "info migrate" like on outgoing side.  Threaded tcp incoming
 migration
 is enable only when --enable-io-thread is set.


>>>
>>> I'm much less confident that threading is the answer here.  We really
>>> would
>>> just need to have asynchronous incoming migration.
>>>
>>
>> You mean, go back to select() in main(), and then call incoming
>> handler each time?
>> Won't it introduce more latency, resulting less throughput?
>>
>
> I wouldn't think so.  With threads, you've got to acquire locks and that
> lock acquisition can be a source of significant latency.  By blocking in
> select, you've got a straight dispatch path.
>
> Really, we'd get 99% of the way there just focusing on live loading of ram.
>  There's really no need to make the final stage of migration live.
>
>> Although threading maybe a big hammer for just getting monitor
>> working, I think using thread for incoming handler may worth if it
>> doesn't heart performance on receiving data.
>>
>
> Unless I'm mistaken, the thread patches you've posted make no attempt at
> addressing the problem of locking.  I believe that properly handling locking
> would result in the patch series increasing in size and complexity rather
> significantly.
>
> I think the simpler approach is to implement a state machine for ram
> loading.

OK.  Making ram loading to be selectable seems to be a good start point.

Thanks,

Yoshi

>
> Regards,
>
> Anthony Liguori
>
>> The downside is mutual exclusion, of course...
>>
>> Thanks,
>>
>> Yoshi
>>
>>
>>>
>>> Regards,
>>>
>>> Anthony Liguori
>>>
>>>

 This series apply on top of patch from Corentin posted on May 29.

 http://www.mail-archive.com/qemu-devel@nongnu.org/msg33830.html

 Yoshiaki Tamura (4):
   qemu-thread: add qemu_thread_join().
   migration-tcp: threaded tcp incoming migration.
   arch_init: calculate transferred bytes at ram_load().
   migration: add support to print migration info on incoming side.

  arch_init.c     |    2 +
  migration-tcp.c |   86
 ++-
  migration.c     |   18 +--
  migration.h     |    2 +-
  qemu-thread.c   |    9 ++
  qemu-thread.h   |    1 +
  6 files changed, 99 insertions(+), 19 deletions(-)



>>>
>>>
>>>
>
>
>



  1   2   >