[Qemu-devel] [PATCH v2 06/15] target-i386: Add "stepping" property to X86CPU

[Andreas Färber]

Signed-off-by: Andreas Färber 
Reviewed-by: Eduardo Habkost 
---
 target-i386/cpu.c |   27 ---
 1 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 3a8141b..2f843be 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -649,10 +649,28 @@ static void x86_cpuid_version_set_model(Object *obj, 
Visitor *v, void *opaque,
 env->cpuid_version |= ((value & 0xf) << 4) | ((value >> 4) << 16);
 }
 
-static void x86_cpuid_version_set_stepping(CPUX86State *env, int stepping)
+static void x86_cpuid_version_set_stepping(Object *obj, Visitor *v,
+   void *opaque, const char *name,
+   Error **errp)
 {
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
+const int64_t min = 0;
+const int64_t max = 0xf;
+int64_t value;
+
+visit_type_int(v, &value, name, errp);
+if (error_is_set(errp)) {
+return;
+}
+if (value < min || value > max) {
+error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "",
+  name ? name : "null", value, min, max);
+return;
+}
+
 env->cpuid_version &= ~0xf;
-env->cpuid_version |= stepping & 0xf;
+env->cpuid_version |= value & 0xf;
 }
 
 static void x86_cpuid_set_model_id(CPUX86State *env, const char *model_id)
@@ -966,7 +984,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 env->cpuid_level = def->level;
 object_property_set_int(OBJECT(cpu), def->family, "family", &error);
 object_property_set_int(OBJECT(cpu), def->model, "model", &error);
-x86_cpuid_version_set_stepping(env, def->stepping);
+object_property_set_int(OBJECT(cpu), def->stepping, "stepping", &error);
 env->cpuid_features = def->features;
 env->cpuid_ext_features = def->ext_features;
 env->cpuid_ext2_features = def->ext2_features;
@@ -1524,6 +1542,9 @@ static void x86_cpu_initfn(Object *obj)
 object_property_add(obj, "model", "int",
 NULL,
 x86_cpuid_version_set_model, NULL, NULL, NULL);
+object_property_add(obj, "stepping", "int",
+NULL,
+x86_cpuid_version_set_stepping, NULL, NULL, NULL);
 
 env->cpuid_apic_id = env->cpu_index;
 mce_init(cpu);
-- 
1.7.7

[Qemu-devel] [PATCH v2 10/15] target-i386: Add property getter for CPU stepping

[Andreas Färber]

Signed-off-by: Andreas Färber 
Reviewed-by: Eduardo Habkost 
---
 target-i386/cpu.c |   14 +-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 643289f..f1d3827 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -675,6 +675,18 @@ static void x86_cpuid_version_set_model(Object *obj, 
Visitor *v, void *opaque,
 env->cpuid_version |= ((value & 0xf) << 4) | ((value >> 4) << 16);
 }
 
+static void x86_cpuid_version_get_stepping(Object *obj, Visitor *v,
+   void *opaque, const char *name,
+   Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
+int64_t value;
+
+value = env->cpuid_version & 0xf;
+visit_type_int(v, &value, name, errp);
+}
+
 static void x86_cpuid_version_set_stepping(Object *obj, Visitor *v,
void *opaque, const char *name,
Error **errp)
@@ -1572,7 +1584,7 @@ static void x86_cpu_initfn(Object *obj)
 x86_cpuid_version_get_model,
 x86_cpuid_version_set_model, NULL, NULL, NULL);
 object_property_add(obj, "stepping", "int",
-NULL,
+x86_cpuid_version_get_stepping,
 x86_cpuid_version_set_stepping, NULL, NULL, NULL);
 object_property_add_str(obj, "model-id",
 NULL,
-- 
1.7.7

[Qemu-devel] [PATCH v2 05/15] target-i386: Add "model" property to X86CPU

[Andreas Färber]

Signed-off-by: Andreas Färber 
Reviewed-by: Eduardo Habkost 
---
 target-i386/cpu.c |   26 +++---
 1 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index ebe9c7e..3a8141b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -626,10 +626,27 @@ static void x86_cpuid_version_set_family(Object *obj, 
Visitor *v, void *opaque,
 }
 }
 
-static void x86_cpuid_version_set_model(CPUX86State *env, int model)
+static void x86_cpuid_version_set_model(Object *obj, Visitor *v, void *opaque,
+const char *name, Error **errp)
 {
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
+const int64_t min = 0;
+const int64_t max = 0xff;
+int64_t value;
+
+visit_type_int(v, &value, name, errp);
+if (error_is_set(errp)) {
+return;
+}
+if (value < min || value > max) {
+error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "",
+  name ? name : "null", value, min, max);
+return;
+}
+
 env->cpuid_version &= ~0xf00f0;
-env->cpuid_version |= ((model & 0xf) << 4) | ((model >> 4) << 16);
+env->cpuid_version |= ((value & 0xf) << 4) | ((value >> 4) << 16);
 }
 
 static void x86_cpuid_version_set_stepping(CPUX86State *env, int stepping)
@@ -948,7 +965,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 env->cpuid_vendor_override = def->vendor_override;
 env->cpuid_level = def->level;
 object_property_set_int(OBJECT(cpu), def->family, "family", &error);
-x86_cpuid_version_set_model(env, def->model);
+object_property_set_int(OBJECT(cpu), def->model, "model", &error);
 x86_cpuid_version_set_stepping(env, def->stepping);
 env->cpuid_features = def->features;
 env->cpuid_ext_features = def->ext_features;
@@ -1504,6 +1521,9 @@ static void x86_cpu_initfn(Object *obj)
 object_property_add(obj, "family", "int",
 NULL,
 x86_cpuid_version_set_family, NULL, NULL, NULL);
+object_property_add(obj, "model", "int",
+NULL,
+x86_cpuid_version_set_model, NULL, NULL, NULL);
 
 env->cpuid_apic_id = env->cpu_index;
 mce_init(cpu);
-- 
1.7.7

[Qemu-devel] [PATCH v2 00/15] QOM'ify x86 CPU, part 2: properties

[Andreas Färber]

Hello,

This series introduces some QOM properties for X86CPU, so that our built-in
init code exercises the same code paths as QMP, as suggested by Eduardo:
* "family",
* "model",
* "stepping" and
* "model-id" (rather than "model_id")
This QOM'ifies my previously introduced helper functions, adding getters.

In the same spirit I've also introduced numeric QOM properties for:
* "level"
* "xlevel"
* "tsc-frequency" (rather than "tsc_freq")

Further I've prepared one QOM property that's currently unused:
* "vendor" (converting three words to string and back seemed too much overhead)

By constrast, the HyperV -cpu property "hv_spinlocks" and flags "hv_relaxed"
and "hv_vapic" do not seem to be per-CPU properties.

v2 fixes minor issues and if I can get an Acked-by or Reviewed-by for 09/15
then I'll send a PULL tomorrow.

Available from:
git://github.com/afaerber/qemu-cpu.git qom-cpu-x86-prop.v2
https://github.com/afaerber/qemu-cpu/commits/qom-cpu-x86-prop.v2

Regards,
Andreas

Cc: Anthony Liguori 
Cc: Jan Kiszka 
Cc: Igor Mammedov 
Cc: Liu Jinsong 
Cc: Lai Jiangshan 
Cc: Vasilis Liaskovitis 
Cc: Eduardo Habkost 
Cc: Michael Roth 
Cc: Paolo Bonzini 
Cc: Vadim Rozenfeld 

Andreas Färber (15):
  target-i386: Fix x86_cpuid_set_model_id()
  target-i386: Pass X86CPU to cpu_x86_register()
  target-i386: Add range check for -cpu ,family=x
  target-i386: Add "family" property to X86CPU
  target-i386: Add "model" property to X86CPU
  target-i386: Add "stepping" property to X86CPU
  target-i386: Add "model-id" property to X86CPU
  target-i386: Add property getter for CPU family
  target-i386: Add property getter for CPU model
  target-i386: Add property getter for CPU stepping
  target-i386: Add property getter for CPU model-id
  target-i386: Introduce "level" property for X86CPU
  target-i386: Introduce "xlevel" property for X86CPU
  target-i386: Prepare "vendor" property for X86CPU
  target-i386: Introduce "tsc-frequency" property for X86CPU

 target-i386/cpu.c|  320 +++---
 target-i386/cpu.h|2 +-
 target-i386/helper.c |2 +-
 3 files changed, 304 insertions(+), 20 deletions(-)

-- 
1.7.7

[Qemu-devel] [PATCH v2 03/15] target-i386: Add range check for -cpu , family=x

[Andreas Färber]

A family field value of 0xf and extended family field value of 0xff is
the maximum representable unsigned family number.
All other CPUID property values are bounds-checked, so add a check here
for symmetry before we adopt it in a property setter.

Signed-off-by: Andreas Färber 
Reviewed-by: Eduardo Habkost 
---
 target-i386/cpu.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e95a1d8..d30185b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -693,7 +693,7 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
 if (!strcmp(featurestr, "family")) {
 char *err;
 numvalue = strtoul(val, &err, 0);
-if (!*val || *err) {
+if (!*val || *err || numvalue > 0xff + 0xf) {
 fprintf(stderr, "bad numerical value %s\n", val);
 goto error;
 }
-- 
1.7.7

Re: [Qemu-devel] [PATCH 1/1] virtio-serial-bus: fix guest_connected init before driver init

[Amit Shah]

On (Tue) 24 Apr 2012 [13:17:16], Michael S. Tsirkin wrote:
> On Tue, Apr 24, 2012 at 02:55:26PM +0530, Amit Shah wrote:
> > From: Alon Levy 
> > 
> > guest_connected should be false before guest driver initialization, and
> > true after, both for multiport aware and non multiport aware drivers.
> > 
> > Don't set it before the guest_features are available; instead use
> > set_status which is called by io to VIRTIO_PCI_STATUS with
> > VIRTIO_CONFIG_S_DRIVER_OK by even older non multiport drivers.
> > 
> > [Amit: Add comment, tweak summary]
> 
> Logic also changed fron Alon's patch. Why?

Yes, forgot to mention that here.

> > Signed-off-by: Alon Levy 
> > Acked-by: Michael S. Tsirkin 
> > Signed-off-by: Amit Shah 
> > ---
> >  hw/virtio-serial-bus.c |   29 +
> >  1 files changed, 21 insertions(+), 8 deletions(-)
> > 
> > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> > index e22940e..796224b 100644
> > --- a/hw/virtio-serial-bus.c
> > +++ b/hw/virtio-serial-bus.c
> > @@ -528,6 +528,26 @@ static void set_config(VirtIODevice *vdev, const 
> > uint8_t *config_data)
> >  memcpy(&config, config_data, sizeof(config));
> >  }
> >  
> > +static void set_status(VirtIODevice *vdev, uint8_t status)
> > +{
> > +VirtIOSerial *vser;
> > +VirtIOSerialPort *port;
> > +
> > +vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
> > +port = find_port_by_id(vser, 0);
> > +
> > +if (port && !use_multiport(port->vser)
> > +&& (status & VIRTIO_CONFIG_S_DRIVER_OK)) {
> > +/*
> > + * Non-multiport guests won't be able to tell us guest
> > + * open/close status.  Such guests can only have a port at id
> > + * 0, so set guest_connected for such ports as soon as guest
> > + * is up.
> > + */
> > +port->guest_connected = true;
> > +}
> > +}
> > +
> 
> Weird.  Don't you want to set guest_connected = false when driver
> is unloaded?
> This is what Alon's patch did:
> port->guest_connected = status & VIRTIO_CONFIG_S_DRIVER_OK;

Setting guest_connected to false when driver is unloaded is something
that's not done before this patch (and not noted in the commit log
too).

And that case isn't specific to just port 0 (in the !multiport case);
all ports will have to be updated for such a change.

Is that something we should do?  It's obviously correct to do that.
Will it affect anything?  I doubt it.  But needs a separate patch and
discussion for that change.

Amit

[Qemu-devel] [PATCH v2 02/15] target-i386: Pass X86CPU to cpu_x86_register()

[Andreas Färber]

Avoids an x86_env_get_cpu() call there, to work with QOM properties.

Signed-off-by: Andreas Färber 
Reviewed-by: Eduardo Habkost 
---
 target-i386/cpu.c|3 ++-
 target-i386/cpu.h|2 +-
 target-i386/helper.c |2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 80c1ca5..e95a1d8 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -907,8 +907,9 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf, 
const char *optarg)
 }
 }
 
-int cpu_x86_register (CPUX86State *env, const char *cpu_model)
+int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 {
+CPUX86State *env = &cpu->env;
 x86_def_t def1, *def = &def1;
 
 memset(def, 0, sizeof(*def));
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 4bb4592..b5b9a50 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -901,7 +901,7 @@ int cpu_x86_signal_handler(int host_signum, void *pinfo,
 void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
uint32_t *eax, uint32_t *ebx,
uint32_t *ecx, uint32_t *edx);
-int cpu_x86_register (CPUX86State *env, const char *cpu_model);
+int cpu_x86_register(X86CPU *cpu, const char *cpu_model);
 void cpu_clear_apic_feature(CPUX86State *env);
 void host_cpuid(uint32_t function, uint32_t count,
 uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 87954f0..0b22582 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1176,7 +1176,7 @@ CPUX86State *cpu_x86_init(const char *cpu_model)
 cpu_set_debug_excp_handler(breakpoint_handler);
 #endif
 }
-if (cpu_x86_register(env, cpu_model) < 0) {
+if (cpu_x86_register(cpu, cpu_model) < 0) {
 object_delete(OBJECT(cpu));
 return NULL;
 }
-- 
1.7.7

[Qemu-devel] [PATCH v2 07/15] target-i386: Add "model-id" property to X86CPU

[Andreas Färber]

Signed-off-by: Andreas Färber 
Reviewed-by: Eduardo Habkost 
---
 target-i386/cpu.c |   10 --
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 2f843be..1b8053a 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -673,8 +673,11 @@ static void x86_cpuid_version_set_stepping(Object *obj, 
Visitor *v,
 env->cpuid_version |= value & 0xf;
 }
 
-static void x86_cpuid_set_model_id(CPUX86State *env, const char *model_id)
+static void x86_cpuid_set_model_id(Object *obj, const char *model_id,
+   Error **errp)
 {
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
 int c, len, i;
 
 if (model_id == NULL) {
@@ -1006,7 +1009,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 env->cpuid_ext3_features &= TCG_EXT3_FEATURES;
 env->cpuid_svm_features &= TCG_SVM_FEATURES;
 }
-x86_cpuid_set_model_id(env, def->model_id);
+object_property_set_str(OBJECT(cpu), def->model_id, "model-id", &error);
 if (error_is_set(&error)) {
 error_free(error);
 return -1;
@@ -1545,6 +1548,9 @@ static void x86_cpu_initfn(Object *obj)
 object_property_add(obj, "stepping", "int",
 NULL,
 x86_cpuid_version_set_stepping, NULL, NULL, NULL);
+object_property_add_str(obj, "model-id",
+NULL,
+x86_cpuid_set_model_id, NULL);
 
 env->cpuid_apic_id = env->cpu_index;
 mce_init(cpu);
-- 
1.7.7

[Qemu-devel] [PATCH v2 15/15] target-i386: Introduce "tsc-frequency" property for X86CPU

[Andreas Färber]

Use Hz as unit.

Signed-off-by: Andreas Färber 
Reviewed-by: Eduardo Habkost 
---
 target-i386/cpu.c |   37 -
 1 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 540b3df..64fd903 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -857,6 +857,37 @@ static void x86_cpuid_set_model_id(Object *obj, const char 
*model_id,
 }
 }
 
+static void x86_cpuid_get_tsc_freq(Object *obj, Visitor *v, void *opaque,
+   const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+int64_t value;
+
+value = cpu->env.tsc_khz * 1000;
+visit_type_int(v, &value, name, errp);
+}
+
+static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
+   const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+const int64_t min = 0;
+const int64_t max = INT_MAX;
+int64_t value;
+
+visit_type_int(v, &value, name, errp);
+if (error_is_set(errp)) {
+return;
+}
+if (value < min || value > max) {
+error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "",
+  name ? name : "null", value, min, max);
+return;
+}
+
+cpu->env.tsc_khz = value / 1000;
+}
+
 static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
 {
 unsigned int i;
@@ -1157,7 +1188,8 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 env->cpuid_svm_features = def->svm_features;
 env->cpuid_ext4_features = def->ext4_features;
 env->cpuid_xlevel2 = def->xlevel2;
-env->tsc_khz = def->tsc_khz;
+object_property_set_int(OBJECT(cpu), (int64_t)def->tsc_khz * 1000,
+"tsc-frequency", &error);
 if (!kvm_enabled()) {
 env->cpuid_features &= TCG_FEATURES;
 env->cpuid_ext_features &= TCG_EXT_FEATURES;
@@ -1720,6 +1752,9 @@ static void x86_cpu_initfn(Object *obj)
 object_property_add_str(obj, "model-id",
 x86_cpuid_get_model_id,
 x86_cpuid_set_model_id, NULL);
+object_property_add(obj, "tsc-frequency", "int",
+x86_cpuid_get_tsc_freq,
+x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
 
 env->cpuid_apic_id = env->cpu_index;
 mce_init(cpu);
-- 
1.7.7

[Qemu-devel] [PULL] virtio-serial: fix probing for features before driver init

[Amit Shah]

Hi,

Please pull for a virtio-serial fix.

The following changes since commit 3c30dd5a68e9fee6af67cfd0d14ed7520820f36a:

  target-arm: Move reset handling to arm_cpu_reset (2012-04-21 18:13:22 +)

are available in the git repository at:
  git://git.kernel.org/pub/scm/virt/qemu/amit/virtio-serial.git master


Alon Levy (1):
  virtio-serial-bus: fix guest_connected init before driver init

 hw/virtio-serial-bus.c |   29 +
 1 files changed, 21 insertions(+), 8 deletions(-)

-- 
1.7.7.6

Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media

[Pavel Hrdina]

On 04/24/2012 11:32 AM, Kevin Wolf wrote:

Am 23.04.2012 18:06, schrieb Pavel Hrdina:

Hi,
this is the patch to fix incorrect handling of IDE floppy drive controller 
emulation
when no media is present. If the guest is booted without a media then the drive
was not being emulated at all but this patch enables the emulation with no 
media present.

There was a bug in FDC emulation without media. Driver was not able to 
recognize that
there is no media in drive.

This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the 
behaviour
is as expected, i.e. as follows:

Linux guest (Fedora 16 x86_64) tries "mount /dev/fd0" and exit with error
"mount: /dev/fd0 is not a valid block device" which is the same behavior like
bare metal with real floppy device (you have to load floppy driver at first
using e.g. "modprobe floppy" command).

For Windows XP guest the Windows floppy driver is trying to seek the virtual 
drive
when you want to open it but driver successfully detect that there is no media 
in drive
and then it's asking user to insert floppy media in the drive.

I also tested behavior of this patch if you start guest with "-nodefaults" and 
both
Windows and Linux guests detect only FDC but no drive.

Pavel

This patch has been written with help of specifications from:
http://www.ousob.com/ng/hardware/ngd127.php
http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
http://wiki.osdev.org/Floppy_Disk_Controller

Signed-off-by: Pavel Hrdina
Signed-off-by: Michal Novotny

It would be cool to have a qtest case for this. But I think we don't
really have a nice way to talk to the qemu monitor yet, so I'm not
requesting this before the patch can go in.


---
  hw/fdc.c |   14 ++
  hw/pc.c  |3 ++-
  2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index a0236b7..6791eff 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -179,12 +179,14 @@ static void fd_revalidate(FDrive *drv)
  FDriveRate rate;

  FLOPPY_DPRINTF("revalidate\n");
-if (drv->bs != NULL&&  bdrv_is_inserted(drv->bs)) {
+if (drv->bs != NULL) {
  ro = bdrv_is_read_only(drv->bs);
  bdrv_get_floppy_geometry_hint(drv->bs,&nb_heads,&max_track,
&last_sect, drv->drive,&drive,&rate);

I'm not sure how your patch works, but I believe the behaviour of
bdrv_get_floppy_geometry_hint might be one of the keys. If I understand
correctly, it will just return the default geometry, which is one for
3.5" 1.44 MB floppies, or more precisely:

{ FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },

Why it makes sense to have a medium geometry when there is no medium I
haven't understood yet, but last_sect/max_track = 0 didn't seem to be
enough for you. Do you know what exactly it is that makes your case work?

ro has undefined value for a BlockDriverState with no medium, but I
guess it doesn't hurt.

This modification is needed for floppy driver in guest to detect floppy drive.


-if (nb_heads != 0&&  max_track != 0&&  last_sect != 0) {
-FLOPPY_DPRINTF("User defined disk (%d %d %d)",
+if (!bdrv_is_inserted(drv->bs)) {
+FLOPPY_DPRINTF("No media in drive\n");
+} else if (nb_heads != 0&&  max_track != 0&&  last_sect != 0) {
+FLOPPY_DPRINTF("User defined disk (%d %d %d)\n",
 nb_heads - 1, max_track, last_sect);
  } else {
  FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n", nb_heads,
@@ -201,11 +203,12 @@ static void fd_revalidate(FDrive *drv)
  drv->drive = drive;
  drv->media_rate = rate;
  } else {
-FLOPPY_DPRINTF("No disk in drive\n");
+FLOPPY_DPRINTF("Drive disabled\n");
  drv->last_sect = 0;
  drv->max_track = 0;
  drv->flags&= ~FDISK_DBL_SIDES;
  }
+
  }
This code is needed to set up default drive geometry so guest can detect 
floppy drive controller.


  //
@@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)

  if (!drv->bs)
  return 0;
+/* This is needed for driver to detect there is no media in drive */
+if (!bdrv_is_inserted(drv->bs))
+return 1;

In which case is this required to detect that there is no media? After
eject? If so, why isn't the code in fdctrl_change_cb() enough?

Or do you in fact need it for the initial state?

As i wrote to Stefan,
You can look here, http://www.isdaman.com/alsos/hardware/fdc/floppy.htm 
, for specification of DIR register. Bit7 is there as CHAN and in this 
bit is saved information whether media is changed or not. This bit is 
set to true while there is no media. And floppy driver is checking this 
bit to detect media change or media missing.


And this is needed for all cases if there is no media in drive. Code in 
fdctrl_change_cb() is needed only for detect that there is no media when 
you try to mount it (linux guest) or open it (windows guest).

Re: [Qemu-devel] [PATCH] spice: require spice-protocol >= 0.8.1

[Gerd Hoffmann]

On 04/23/12 15:52, Peter Maydell wrote:
> Ping? This patch doesn't seem to have made it into master yet
> and I don't think it was in the last spice pullreq...

Somehow overlooked it in my inbox.  Added to the spice patch queue now.

Thanks for the reminder,
  Gerd

Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media

[Kevin Wolf]

Am 23.04.2012 18:06, schrieb Pavel Hrdina:
> Hi,
> this is the patch to fix incorrect handling of IDE floppy drive controller 
> emulation
> when no media is present. If the guest is booted without a media then the 
> drive
> was not being emulated at all but this patch enables the emulation with no 
> media present.
> 
> There was a bug in FDC emulation without media. Driver was not able to 
> recognize that
> there is no media in drive.
> 
> This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the 
> behaviour
> is as expected, i.e. as follows:
> 
> Linux guest (Fedora 16 x86_64) tries "mount /dev/fd0" and exit with error
> "mount: /dev/fd0 is not a valid block device" which is the same behavior like
> bare metal with real floppy device (you have to load floppy driver at first
> using e.g. "modprobe floppy" command).
> 
> For Windows XP guest the Windows floppy driver is trying to seek the virtual 
> drive
> when you want to open it but driver successfully detect that there is no 
> media in drive
> and then it's asking user to insert floppy media in the drive.
> 
> I also tested behavior of this patch if you start guest with "-nodefaults" 
> and both
> Windows and Linux guests detect only FDC but no drive.
> 
> Pavel
> 
> This patch has been written with help of specifications from:
> http://www.ousob.com/ng/hardware/ngd127.php
> http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
> http://wiki.osdev.org/Floppy_Disk_Controller
> 
> Signed-off-by: Pavel Hrdina 
> Signed-off-by: Michal Novotny 

It would be cool to have a qtest case for this. But I think we don't
really have a nice way to talk to the qemu monitor yet, so I'm not
requesting this before the patch can go in.

> ---
>  hw/fdc.c |   14 ++
>  hw/pc.c  |3 ++-
>  2 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/fdc.c b/hw/fdc.c
> index a0236b7..6791eff 100644
> --- a/hw/fdc.c
> +++ b/hw/fdc.c
> @@ -179,12 +179,14 @@ static void fd_revalidate(FDrive *drv)
>  FDriveRate rate;
>  
>  FLOPPY_DPRINTF("revalidate\n");
> -if (drv->bs != NULL && bdrv_is_inserted(drv->bs)) {
> +if (drv->bs != NULL) {
>  ro = bdrv_is_read_only(drv->bs);
>  bdrv_get_floppy_geometry_hint(drv->bs, &nb_heads, &max_track,
>&last_sect, drv->drive, &drive, &rate);

I'm not sure how your patch works, but I believe the behaviour of
bdrv_get_floppy_geometry_hint might be one of the keys. If I understand
correctly, it will just return the default geometry, which is one for
3.5" 1.44 MB floppies, or more precisely:

{ FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },

Why it makes sense to have a medium geometry when there is no medium I
haven't understood yet, but last_sect/max_track = 0 didn't seem to be
enough for you. Do you know what exactly it is that makes your case work?

ro has undefined value for a BlockDriverState with no medium, but I
guess it doesn't hurt.

> -if (nb_heads != 0 && max_track != 0 && last_sect != 0) {
> -FLOPPY_DPRINTF("User defined disk (%d %d %d)",
> +if (!bdrv_is_inserted(drv->bs)) {
> +FLOPPY_DPRINTF("No media in drive\n");
> +} else if (nb_heads != 0 && max_track != 0 && last_sect != 0) {
> +FLOPPY_DPRINTF("User defined disk (%d %d %d)\n",
> nb_heads - 1, max_track, last_sect);
>  } else {
>  FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n", nb_heads,
> @@ -201,11 +203,12 @@ static void fd_revalidate(FDrive *drv)
>  drv->drive = drive;
>  drv->media_rate = rate;
>  } else {
> -FLOPPY_DPRINTF("No disk in drive\n");
> +FLOPPY_DPRINTF("Drive disabled\n");
>  drv->last_sect = 0;
>  drv->max_track = 0;
>  drv->flags &= ~FDISK_DBL_SIDES;
>  }
> +
>  }
>  
>  //
> @@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
>  
>  if (!drv->bs)
>  return 0;
> +/* This is needed for driver to detect there is no media in drive */
> +if (!bdrv_is_inserted(drv->bs))
> +return 1;

In which case is this required to detect that there is no media? After
eject? If so, why isn't the code in fdctrl_change_cb() enough?

Or do you in fact need it for the initial state?

Kevin

[Qemu-devel] [PATCH v2 14/15] target-i386: Prepare "vendor" property for X86CPU

[Andreas Färber]

Using it now would incur converting the three x86_def_t vendor words
into a string for object_property_set_str(), then back to three words
in the "vendor" setter.
The built-in CPU definitions use numeric preprocessor defines to
initialize the three words in a charset-safe way, so do not change the
fields to char[12] just to use the setter.

Signed-off-by: Andreas Färber 
Reviewed-by: Eduardo Habkost 
---
 target-i386/cpu.c |   44 
 1 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 89a1855..540b3df 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -777,6 +777,47 @@ static void x86_cpuid_set_xlevel(Object *obj, Visitor *v, 
void *opaque,
 cpu->env.cpuid_xlevel = value;
 }
 
+static char *x86_cpuid_get_vendor(Object *obj, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
+char *value;
+int i;
+
+value = (char *)g_malloc(12 + 1);
+for (i = 0; i < 4; i++) {
+value[i] = env->cpuid_vendor1 >> (8 * i);
+value[i + 4] = env->cpuid_vendor2 >> (8 * i);
+value[i + 8] = env->cpuid_vendor3 >> (8 * i);
+}
+value[12] = '\0';
+return value;
+}
+
+static void x86_cpuid_set_vendor(Object *obj, const char *value,
+ Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
+int i;
+
+if (strlen(value) != 12) {
+error_set(errp, QERR_PROPERTY_VALUE_BAD, "",
+  "vendor", value);
+return;
+}
+
+env->cpuid_vendor1 = 0;
+env->cpuid_vendor2 = 0;
+env->cpuid_vendor3 = 0;
+for (i = 0; i < 4; i++) {
+env->cpuid_vendor1 |= ((uint8_t)value[i]) << (8 * i);
+env->cpuid_vendor2 |= ((uint8_t)value[i + 4]) << (8 * i);
+env->cpuid_vendor3 |= ((uint8_t)value[i + 8]) << (8 * i);
+}
+env->cpuid_vendor_override = 1;
+}
+
 static char *x86_cpuid_get_model_id(Object *obj, Error **errp)
 {
 X86CPU *cpu = X86_CPU(obj);
@@ -1673,6 +1714,9 @@ static void x86_cpu_initfn(Object *obj)
 object_property_add(obj, "xlevel", "int",
 x86_cpuid_get_xlevel,
 x86_cpuid_set_xlevel, NULL, NULL, NULL);
+object_property_add_str(obj, "vendor",
+x86_cpuid_get_vendor,
+x86_cpuid_set_vendor, NULL);
 object_property_add_str(obj, "model-id",
 x86_cpuid_get_model_id,
 x86_cpuid_set_model_id, NULL);
-- 
1.7.7

[Qemu-devel] [PATCH v2 04/15] target-i386: Add "family" property to X86CPU

[Andreas Färber]

Add the property early in the initfn so that it can be used in helpers
such as mce_init().

Signed-off-by: Andreas Färber 
Reviewed-by: Eduardo Habkost 
[AF: Add an error_free(), spotted by Michael Roth]
---
 target-i386/cpu.c |   39 ++-
 1 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index d30185b..ebe9c7e 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -27,6 +27,8 @@
 #include "qemu-option.h"
 #include "qemu-config.h"
 
+#include "qapi/qapi-visit-core.h"
+
 #include "hyperv.h"
 
 /* feature flags taken from "Intel Processor Identification and the CPUID
@@ -597,13 +599,30 @@ static int check_features_against_host(x86_def_t 
*guest_def)
 return rv;
 }
 
-static void x86_cpuid_version_set_family(CPUX86State *env, int family)
+static void x86_cpuid_version_set_family(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
 {
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
+const int64_t min = 0;
+const int64_t max = 0xff + 0xf;
+int64_t value;
+
+visit_type_int(v, &value, name, errp);
+if (error_is_set(errp)) {
+return;
+}
+if (value < min || value > max) {
+error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "",
+  name ? name : "null", value, min, max);
+return;
+}
+
 env->cpuid_version &= ~0xff00f00;
-if (family > 0x0f) {
-env->cpuid_version |= 0xf00 | ((family - 0x0f) << 20);
+if (value > 0x0f) {
+env->cpuid_version |= 0xf00 | ((value - 0x0f) << 20);
 } else {
-env->cpuid_version |= family << 8;
+env->cpuid_version |= value << 8;
 }
 }
 
@@ -911,6 +930,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 {
 CPUX86State *env = &cpu->env;
 x86_def_t def1, *def = &def1;
+Error *error = NULL;
 
 memset(def, 0, sizeof(*def));
 
@@ -927,7 +947,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 }
 env->cpuid_vendor_override = def->vendor_override;
 env->cpuid_level = def->level;
-x86_cpuid_version_set_family(env, def->family);
+object_property_set_int(OBJECT(cpu), def->family, "family", &error);
 x86_cpuid_version_set_model(env, def->model);
 x86_cpuid_version_set_stepping(env, def->stepping);
 env->cpuid_features = def->features;
@@ -952,6 +972,10 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 env->cpuid_svm_features &= TCG_SVM_FEATURES;
 }
 x86_cpuid_set_model_id(env, def->model_id);
+if (error_is_set(&error)) {
+error_free(error);
+return -1;
+}
 return 0;
 }
 
@@ -1476,6 +1500,11 @@ static void x86_cpu_initfn(Object *obj)
 CPUX86State *env = &cpu->env;
 
 cpu_exec_init(env);
+
+object_property_add(obj, "family", "int",
+NULL,
+x86_cpuid_version_set_family, NULL, NULL, NULL);
+
 env->cpuid_apic_id = env->cpu_index;
 mce_init(cpu);
 }
-- 
1.7.7

Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media

[Pavel Hrdina]

On 04/24/2012 11:06 AM, Stefan Hajnoczi wrote:

On Mon, Apr 23, 2012 at 5:06 PM, Pavel Hrdina  wrote:

Hi,
this is the patch to fix incorrect handling of IDE floppy drive controller 
emulation

s/IDE//

It's unrelated to IDE.


@@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)

 if (!drv->bs)
 return 0;
+/* This is needed for driver to detect there is no media in drive */
+if (!bdrv_is_inserted(drv->bs))
+return 1;
 if (drv->media_changed) {
 drv->media_changed = 0;
 ret = 1;

Why isn't the BlockDevOps.change_media_cb() mechanism enough to report
disk changes correctly?

Stefan
You can look here, http://www.isdaman.com/alsos/hardware/fdc/floppy.htm 
, for specification of DIR register. Bit7 is there as CHAN and in this 
bit is saved information whether media is changed or not. This bit is 
set to true while there is no media. And floppy driver is checking this 
bit to detect media change or media missing.

[Qemu-devel] [PATCH v2 12/15] target-i386: Introduce "level" property for X86CPU

[Andreas Färber]

Signed-off-by: Andreas Färber 
Reviewed-by: Eduardo Habkost 
---
 target-i386/cpu.c |   38 +-
 1 files changed, 37 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 490db76..f2df8d0 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -711,6 +711,39 @@ static void x86_cpuid_version_set_stepping(Object *obj, 
Visitor *v,
 env->cpuid_version |= value & 0xf;
 }
 
+static void x86_cpuid_get_level(Object *obj, Visitor *v, void *opaque,
+const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+int64_t value;
+
+value = cpu->env.cpuid_level;
+/* TODO Use visit_type_uint32() once available */
+visit_type_int(v, &value, name, errp);
+}
+
+static void x86_cpuid_set_level(Object *obj, Visitor *v, void *opaque,
+const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+const int64_t min = 0;
+const int64_t max = UINT32_MAX;
+int64_t value;
+
+/* TODO Use visit_type_uint32() once available */
+visit_type_int(v, &value, name, errp);
+if (error_is_set(errp)) {
+return;
+}
+if (value < min || value > max) {
+error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "",
+  name ? name : "null", value, min, max);
+return;
+}
+
+cpu->env.cpuid_level = value;
+}
+
 static char *x86_cpuid_get_model_id(Object *obj, Error **errp)
 {
 X86CPU *cpu = X86_CPU(obj);
@@ -1037,7 +1070,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 env->cpuid_vendor3 = CPUID_VENDOR_INTEL_3;
 }
 env->cpuid_vendor_override = def->vendor_override;
-env->cpuid_level = def->level;
+object_property_set_int(OBJECT(cpu), def->level, "level", &error);
 object_property_set_int(OBJECT(cpu), def->family, "family", &error);
 object_property_set_int(OBJECT(cpu), def->model, "model", &error);
 object_property_set_int(OBJECT(cpu), def->stepping, "stepping", &error);
@@ -1601,6 +1634,9 @@ static void x86_cpu_initfn(Object *obj)
 object_property_add(obj, "stepping", "int",
 x86_cpuid_version_get_stepping,
 x86_cpuid_version_set_stepping, NULL, NULL, NULL);
+object_property_add(obj, "level", "int",
+x86_cpuid_get_level,
+x86_cpuid_set_level, NULL, NULL, NULL);
 object_property_add_str(obj, "model-id",
 x86_cpuid_get_model_id,
 x86_cpuid_set_model_id, NULL);
-- 
1.7.7

[Qemu-devel] [PATCH v2 09/15] target-i386: Add property getter for CPU model

[Andreas Färber]

Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |   14 +-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 9479717..643289f 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -640,6 +640,18 @@ static void x86_cpuid_version_set_family(Object *obj, 
Visitor *v, void *opaque,
 }
 }
 
+static void x86_cpuid_version_get_model(Object *obj, Visitor *v, void *opaque,
+const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
+int64_t value;
+
+value = (env->cpuid_version >> 4) & 0xf;
+value |= ((env->cpuid_version >> 16) & 0xf) << 4;
+visit_type_int(v, &value, name, errp);
+}
+
 static void x86_cpuid_version_set_model(Object *obj, Visitor *v, void *opaque,
 const char *name, Error **errp)
 {
@@ -1557,7 +1569,7 @@ static void x86_cpu_initfn(Object *obj)
 x86_cpuid_version_get_family,
 x86_cpuid_version_set_family, NULL, NULL, NULL);
 object_property_add(obj, "model", "int",
-NULL,
+x86_cpuid_version_get_model,
 x86_cpuid_version_set_model, NULL, NULL, NULL);
 object_property_add(obj, "stepping", "int",
 NULL,
-- 
1.7.7

[Qemu-devel] [PATCH 1/1] virtio-serial-bus: fix guest_connected init before driver init

[Amit Shah]

From: Alon Levy 

guest_connected should be false before guest driver initialization, and
true after, both for multiport aware and non multiport aware drivers.

Don't set it before the guest_features are available; instead use
set_status which is called by io to VIRTIO_PCI_STATUS with
VIRTIO_CONFIG_S_DRIVER_OK by even older non multiport drivers.

[Amit: Add comment, tweak summary]

Signed-off-by: Alon Levy 
Acked-by: Michael S. Tsirkin 
Signed-off-by: Amit Shah 
---
 hw/virtio-serial-bus.c |   29 +
 1 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index e22940e..796224b 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -528,6 +528,26 @@ static void set_config(VirtIODevice *vdev, const uint8_t 
*config_data)
 memcpy(&config, config_data, sizeof(config));
 }
 
+static void set_status(VirtIODevice *vdev, uint8_t status)
+{
+VirtIOSerial *vser;
+VirtIOSerialPort *port;
+
+vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
+port = find_port_by_id(vser, 0);
+
+if (port && !use_multiport(port->vser)
+&& (status & VIRTIO_CONFIG_S_DRIVER_OK)) {
+/*
+ * Non-multiport guests won't be able to tell us guest
+ * open/close status.  Such guests can only have a port at id
+ * 0, so set guest_connected for such ports as soon as guest
+ * is up.
+ */
+port->guest_connected = true;
+}
+}
+
 static void virtio_serial_save(QEMUFile *f, void *opaque)
 {
 VirtIOSerial *s = opaque;
@@ -798,14 +818,6 @@ static int virtser_port_qdev_init(DeviceState *qdev)
 return ret;
 }
 
-if (!use_multiport(port->vser)) {
-/*
- * Allow writes to guest in this case; we have no way of
- * knowing if a guest port is connected.
- */
-port->guest_connected = true;
-}
-
 port->elem.out_num = 0;
 
 QTAILQ_INSERT_TAIL(&port->vser->ports, port, next);
@@ -905,6 +917,7 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, 
virtio_serial_conf *conf)
 vser->vdev.get_features = get_features;
 vser->vdev.get_config = get_config;
 vser->vdev.set_config = set_config;
+vser->vdev.set_status = set_status;
 
 vser->qdev = dev;
 
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH 00/16] QEMU vhost-scsi support

[Stefan Hajnoczi]

On Tue, Apr 24, 2012 at 8:05 AM, Paolo Bonzini  wrote:
> Il 24/04/2012 06:21, ronnie sahlberg ha scritto:
>> Hi Stefan,
>>
>> A little bit off-topic but
>>
>> When you design the proper place and API to plug virt-scsi into an
>> external SCSI parser outside of qemu like the target in the kernel ...
>>
>> It would be very nice if one could also plug virt-scsi into libiscsi
>> and pass the CDBs straight to the remote iSCSI target too.
>> Keep some thoughts on virt-scsi + libiscsi integration.
>
> Yes, that makes a lot of sense.  It's a bit harder than scsi-generic but
> we do want to get there.

Yep.  I think previously there was discussion about a libiscsi
SCSIDevice so that guest SCSI commands can be sent to libiscsi LUNs
without going through the QEMU block layer.  (Another way to pass
arbitrary SCSI commands to libiscsi is by hooking up .bdrv_aio_ioctl()
with SG_IO scsi-generic compatible code in block/iscsi.c.)

I think what you're describing now is one level higher: the libiscsi
target *is* the virtual SCSI target and virtio-scsi just ships off
commands to it?  This would be like a SCSIBus that is handle by
libiscsi target instead of emulated in QEMU.

Stefan

Re: [Qemu-devel] [PATCH 2/4] block: use Error mechanism instead of -errno for block_job_set_speed()

[Paolo Bonzini]

Il 24/04/2012 10:49, Stefan Hajnoczi ha scritto:
> "The error is specific to
> block job speeds so we can add a speed argument to block-stream in the
> future and clearly identify the invalid parameter."
> 
> I added the new error to avoid having to change the InvalidParameter
> 'name' field.  It becomes ugly because we have:
> block-job-set-speed  
> block-stream  [] []
> 
> Notice that the parameter is called 'value' in block-job-set-speed and
> 'speed' in block-stream.  Therefore we can't just propagate a single
> InvalidParameter name='value' error up from block-stream.
> 
> This means that QMP commands will need to change the error to use the
> correct name :(.  It's doable, but ugly and a bit more complex.

Well, we still have time to change block-job-set-speed.  Ugly, but doable.

Paolo

Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media

[Stefan Hajnoczi]

On Mon, Apr 23, 2012 at 5:06 PM, Pavel Hrdina  wrote:
> Hi,
> this is the patch to fix incorrect handling of IDE floppy drive controller 
> emulation

s/IDE//

It's unrelated to IDE.

> @@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
>
>     if (!drv->bs)
>         return 0;
> +    /* This is needed for driver to detect there is no media in drive */
> +    if (!bdrv_is_inserted(drv->bs))
> +        return 1;
>     if (drv->media_changed) {
>         drv->media_changed = 0;
>         ret = 1;

Why isn't the BlockDevOps.change_media_cb() mechanism enough to report
disk changes correctly?

Stefan

Re: [Qemu-devel] Generating cachegrind output with qemu

[Wacha Gábor]

Dear Stefan,

Thanks for your quick answer, I'll look into it.

Regards,
Gabor Wacha
2012.04.24. 6:33, "Stefan Weil"  ezt írta:

> Am 23.04.2012 18:18, schrieb Wacha Gábor:
>
>> Dear developers, I am a Hungarian student trying to use qemu for
>> profiling bare metal ARM programs for my student research. On the following
>> page, it is mentioned that one can generate cachegrind output with qemu:
>> http://www.monstr.eu/wiki/**doku.php?id=qemu:qemu#run_**with_cachegrindUnfortunately
>>  it does not work (altough I checked out the sources from the
>> mentioned git repository), and I found out (using find and grep utilities),
>> that cachegrind is not even mentioned in the qemu sources. My question is:
>> does this feature exist? If the answer is yes, how can I use it? (And I am
>> sorry for my English.) Thanks, Gabor Wacha
>>
>
> Your English is good - much better than my Hungarian :-)
>
> Standard QEMU cannot produce cachegrind output.
>
> According to this mail, a derived version of QEMU supports
> cachegrind output for CRIS and MicroBlaze emulation:
>
> http://lists.gnu.org/archive/**html/qemu-devel/2009-06/**msg01415.html
>
> I cc'ed the author, Edgar Iglesias. Here are the sources:
>
> http://repo.or.cz/w/qemu/cris-**port.git
>
> Cachegrind for ARM emulation is not supported and
> would have to be implemented in QEMU.
>
> Profiling could also be done by using the TCG interpreter
> and extending the interpreter main loop tcg_qemu_tb_exec
> in tci.c.
>
> Regards,
>
> Stefan Weil
>
>

Re: [Qemu-devel] [PATCH 00/15] QOM'ify x86 CPU, part 2: properties

[Andreas Färber]

Am 19.04.2012 20:27, schrieb Eduardo Habkost:
> By the way, do you still plan to make cpudefs register new
> classes/types? I remember that you did that on a previous series.

Generally I do, yes. However the CPU QOM'ification is not making as much
progress as I would've liked, specifically there's still five targets
left in my queue for base QOM'ification for 1.1.

This series, x86 part 2, goes further and prepares properties a) for
general inspection and modification (but without having the CPU exposed
as a child yet), b) for use in instantiating subclasses in part 3. It
was intended for 1.1.

Question is, do we want CPU subclasses for 1.1? ARM has them now but we
won't manage to unify this across targets in time for the Hard Freeze.

For ARM there was a dislike of the ARMCPUClass-based approach and a
preference towards hardcoding things imperatively in initfns. I don't
see how that would work for cpudef, so I would stick with extending
X86CPUClass.

There was a dislike of duplicating fields between X86CPUInfo and
X86CPUClass. However to avoid incurring a sub-struct access the only way
would be to move the field definitions to a macro and use it in both
locations. Further opinions or suggestions welcome.

I've also not yet seen a discussion whether we need to allow modifying
built-in classes via cpudef or whether adding new classes is sufficient.
I had implemented both in my RFC but would prefer the latter if there is
agreement.

> Is it possible to have property get/setters for ObjectClass QOM objects,
> too? It would be interesting to use QOM properties for the cpudef fields
> as well (it would make the work of defining boolean feature fields much
> simpler).

No. Classes do not have property infrastructure and I personally see
them as immutable. cpudef is independent of classes though, so you could
invent a new mechanism to set/unset features, whatever the backend
they'll be written to.

> Reviewed-by: Eduardo Habkost 

Thanks, will send out v2 shortly with the requested changes.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 2/4] block: use Error mechanism instead of -errno for block_job_set_speed()

[Stefan Hajnoczi]

On Mon, Apr 23, 2012 at 7:01 PM, Luiz Capitulino  wrote:
> On Mon, 23 Apr 2012 17:47:09 +0200
> Paolo Bonzini  wrote:
>
>> Il 23/04/2012 17:39, Stefan Hajnoczi ha scritto:
>> > There are at least two different errors that can occur in
>> > block_job_set_speed(): the job might not support setting speeds or the
>> > value might be invalid.
>> >
>> > Use the Error mechanism to report the error where it occurs.  This patch
>> > adds the new BlockJobSpeedInvalid QError.  The error is specific to
>> > block job speeds so we can add a speed argument to block-stream in the
>> > future and clearly identify the invalid parameter.
>> >
>> > Cc: Luiz Capitulino 
>> > Signed-off-by: Stefan Hajnoczi 
>> > ---
>> >  block.c          |   17 ++---
>> >  block/stream.c   |    6 +++---
>> >  block_int.h      |    5 +++--
>> >  blockdev.c       |    4 +---
>> >  qapi-schema.json |    1 +
>> >  qerror.c         |    4 
>> >  qerror.h         |    3 +++
>> >  7 files changed, 25 insertions(+), 15 deletions(-)
>> >
>> > diff --git a/block.c b/block.c
>> > index 528b696..7056d8c 100644
>> > --- a/block.c
>> > +++ b/block.c
>> > @@ -4103,18 +4103,21 @@ void block_job_complete(BlockJob *job, int ret)
>> >      bdrv_set_in_use(bs, 0);
>> >  }
>> >
>> > -int block_job_set_speed(BlockJob *job, int64_t value)
>> > +void block_job_set_speed(BlockJob *job, int64_t value, Error **errp)
>> >  {
>> > -    int rc;
>> > +    Error *local_error = NULL;
>> >
>> >      if (!job->job_type->set_speed) {
>> > -        return -ENOTSUP;
>> > +        error_set(errp, QERR_NOT_SUPPORTED);
>> > +        return;
>> >      }
>> > -    rc = job->job_type->set_speed(job, value);
>> > -    if (rc == 0) {
>> > -        job->speed = value;
>> > +    job->job_type->set_speed(job, value, &local_error);
>> > +    if (error_is_set(&local_error)) {
>> > +        error_propagate(errp, local_error);
>> > +        return;
>> >      }
>> > -    return rc;
>> > +
>> > +    job->speed = value;
>>
>> I don't think this is the right place to add Error handling.  By doing
>> it at QAPI entry points we can reuse an existing error such as
>> QERR_INVALID_PARAMETER_VALUE.
>
> I think the place were we call error_set() isn't a problem. Actually, the 
> Error
> object was specifically designed to be used from qemu depths and be 
> propagated up.
>
> Now:
>
>> If we need to introduce a specific error for each parameter type, we
>> will end up with N errors per function.
>
> I agree with that, QError design induces people to add new errors... Why can't
> you use one of the invalid parameter errors we have?

"The error is specific to
block job speeds so we can add a speed argument to block-stream in the
future and clearly identify the invalid parameter."

I added the new error to avoid having to change the InvalidParameter
'name' field.  It becomes ugly because we have:
block-job-set-speed  
block-stream  [] []

Notice that the parameter is called 'value' in block-job-set-speed and
'speed' in block-stream.  Therefore we can't just propagate a single
InvalidParameter name='value' error up from block-stream.

This means that QMP commands will need to change the error to use the
correct name :(.  It's doable, but ugly and a bit more complex.

Thoughts?

Stefan

Re: [Qemu-devel] copy benchmarks onto qemu

[Mulyadi Santosa]

On Tue, Apr 24, 2012 at 02:09, Xin Tong  wrote:
> I am not too sure what you mean by raw image. what i have is an *.img
> file that is bootable by QEMU. will kpartx  work ?

try to use "qemu-img info" to find what format the file uses...


-- 
regards,

Mulyadi Santosa
Freelance Linux trainer and consultant

blog: the-hydra.blogspot.com
training: mulyaditraining.blogspot.com

Re: [Qemu-devel] [RFC PATCH 0/9] ACPI memory hotplug

[Gleb Natapov]

On Tue, Apr 24, 2012 at 10:24:51AM +0200, Vasilis Liaskovitis wrote:
> Hi,
> On Tue, Apr 24, 2012 at 10:52:24AM +0300, Gleb Natapov wrote:
> > On Mon, Apr 23, 2012 at 02:31:15PM +0200, Vasilis Liaskovitis wrote:
> > > The 440fx spec mentions: "The address range from the top of main DRAM to 4
> > > Gbytes (top of physical memory space supported by the 440FX PCIset) is 
> > > normally
> > > mapped to PCI. The PMC forwards all accesses within this address range to 
> > > PCI."
> > > 
> > > What we probably want is that the initial memory map creation takes into 
> > > account
> > > all dimms specified (both populated/unpopulated) 
> > Yes.
> > 
> > > So "-m 1G, -device dimm,size=1G,populated=true -device 
> > > dimm,size=1G,populated=false"
> > > would create a system map with top of memory and start of PCI-hole at 2G. 
> > > 
> > What -m 1G means on this command line? Isn't it redundant?
> yes, this was redundant with the original concept.
> 
> > May be we should make -m create non unplaggable, populated slot starting
> > at address 0. Ten you config above will specify 3G memory with 2G
> > populated (first of which is not removable) and 1G unpopulated. PCI hole
> > starts above 3G.
> 
> I agree -m should mean one big unpluggable slot.
> 
> So in the new proposal,"-device dimm populated=true" means a hot-removable 
> dimm
> that has already been hotplugged. 
> 
Yes.

> A question here is when exactly should the initial hot-add event for this dimm
> be played? If the relevant OSPM has not yet been initialized (e.g. 
> acpi_memhotplug
> module in a linux guest needs to be loaded), the guest may not see the event. 
> This is a general issue of course, but with initially populated hot-removable
> dimms it may be a bigger issue. Can ospm acpi initialization be detected?
> 
> Or maybe you are suggesting "populated=true" is part of initial memory (i.e. 
> not
> hot-added, but still hot-removable). Though in that case guestOS may use it 
> for
> bootmem allocations, making hot-remove more likely to fail at the memory
> offlining stage.
>
If memory slot is populated even before OSPM is started BIOS will detect
that by reading mem_sts and will create e820 map appropriately. OSPM
will detect it by evaluating _STA during boot. This is not unique for
memory hot-plug. Any hotpluggable device have the same issue.

--
Gleb.

Re: [Qemu-devel] [RFC PATCH 3/9][SeaBIOS] acpi: generate hotplug memory devices.

[Vasilis Liaskovitis]

Hi,

On Mon, Apr 23, 2012 at 07:37:51PM -0400, Kevin O'Connor wrote:
> On Thu, Apr 19, 2012 at 04:08:41PM +0200, Vasilis Liaskovitis wrote:
> >  The memory device generation is guided by qemu paravirt info. Seabios
> >  first uses the info to setup SRAT entries for the hotplug-able memory 
> > slots.
> >  Afterwards, build_memssdt uses the created SRAT entries to generate
> >  appropriate memory device objects. One memory device (and corresponding 
> > SRAT
> >  entry) is generated for each hotplug-able qemu memslot. Currently no SSDT
> >  memory device is created for initial system memory (the method can be
> >  generalized to all memory though).
> > 
> >  Signed-off-by: Vasilis Liaskovitis 
> > ---
> >  src/acpi.c |  151 
> > ++--
> >  1 files changed, 147 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/acpi.c b/src/acpi.c
> > index 30888b9..5580099 100644
> > --- a/src/acpi.c
> > +++ b/src/acpi.c
> > @@ -484,6 +484,131 @@ build_ssdt(void)
> >  return ssdt;
> >  }
> >  
> > +static unsigned char ssdt_mem[] = {
> > +0x5b,0x82,0x47,0x07,0x4d,0x50,0x41,0x41,
> 
> This patch looks like it uses the SSDT generation mechanism that was
> present in SeaBIOS v1.6.3.  Since then, however, the runtime AML code
> generation has been improved to be more dynamic.  Any runtime
> generated AML code should be updated to use the newer mechanisms.

thanks, I will look into the new mechanism and rewrite.

- Vasilis

Re: [Qemu-devel] [RFC PATCH 0/9] ACPI memory hotplug

[Vasilis Liaskovitis]

Hi,
On Tue, Apr 24, 2012 at 10:52:24AM +0300, Gleb Natapov wrote:
> On Mon, Apr 23, 2012 at 02:31:15PM +0200, Vasilis Liaskovitis wrote:
> > The 440fx spec mentions: "The address range from the top of main DRAM to 4
> > Gbytes (top of physical memory space supported by the 440FX PCIset) is 
> > normally
> > mapped to PCI. The PMC forwards all accesses within this address range to 
> > PCI."
> > 
> > What we probably want is that the initial memory map creation takes into 
> > account
> > all dimms specified (both populated/unpopulated) 
> Yes.
> 
> > So "-m 1G, -device dimm,size=1G,populated=true -device 
> > dimm,size=1G,populated=false"
> > would create a system map with top of memory and start of PCI-hole at 2G. 
> > 
> What -m 1G means on this command line? Isn't it redundant?
yes, this was redundant with the original concept.

> May be we should make -m create non unplaggable, populated slot starting
> at address 0. Ten you config above will specify 3G memory with 2G
> populated (first of which is not removable) and 1G unpopulated. PCI hole
> starts above 3G.

I agree -m should mean one big unpluggable slot.

So in the new proposal,"-device dimm populated=true" means a hot-removable dimm
that has already been hotplugged. 

A question here is when exactly should the initial hot-add event for this dimm
be played? If the relevant OSPM has not yet been initialized (e.g. 
acpi_memhotplug
module in a linux guest needs to be loaded), the guest may not see the event. 
This is a general issue of course, but with initially populated hot-removable
dimms it may be a bigger issue. Can ospm acpi initialization be detected?

Or maybe you are suggesting "populated=true" is part of initial memory (i.e. not
hot-added, but still hot-removable). Though in that case guestOS may use it for
bootmem allocations, making hot-remove more likely to fail at the memory
offlining stage.

> 
> > This may require some shifting of physical address offsets around
> > 3.5GB-4GB - is this the minimum PCI hole allowed?
> Currently it is 1G in QEMU code.
ok

> > 
> > E.g. if we specify 4x1GB DIMMs (onlt the first initially populated)   
> > -m 1G, -device dimm,size=1G,populated=true -device 
> > dimm,size=1G,populated=false
> > -device dimm,size=1G,populated=false -device dimm,size=1G,populated=false
> > 
> > we create the following memory map:
> > dimm0: [0,1G)
> > dimm1: [1G, 2G)
> > dimm2: [2G, 3G)
> > dimm3: [4G, 5G) or dimm3 is split into [3G, 3.5G) and [4G, 4.5G)
> > 
> > does either of these options sound reasonable?
> > 
> We shouldn't split dimms IMO. Just unnecessary complication. Better make
> bigger PCI hole.

ok

thanks,

- Vasilis

Re: [Qemu-devel] [PATCH] block/qcow2: Add missing GCC_FMT_ATTR to function report_unsupported()

[Kevin Wolf]

Am 23.04.2012 22:54, schrieb Stefan Weil:
> Cc: Kevin Wolf 
> Signed-off-by: Stefan Weil 
> ---
>  block/qcow2.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index ad46c03..d03e31c 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -182,7 +182,7 @@ static void cleanup_unknown_header_ext(BlockDriverState 
> *bs)
>  }
>  }
>  
> -static void report_unsupported(BlockDriverState *bs, const char *fmt, ...)
> +static void GCC_FMT_ATTR(2, 3) report_unsupported(BlockDriverState *bs, 
> const char *fmt, ...)
>  {
>  char msg[64];
>  va_list ap;

Thanks, fixed to have not more than 80 characters per line and applied
to the block branch.

Kevin

Re: [Qemu-devel] [PATCH v6 0/5] FreeSCALE i.MX31 support

[Peter Chubb]

> "Andreas" == Andreas Färber  writes:

Andreas> Am 23.04.2012 01:31, schrieb Peter Chubb:
>> Hi all, Most of the files are unchanged since last time.

Andreas> Indeed... On v5 I had asked you to shorten the subjects to
Andreas> conform to our commit message scheme and to make patches
Andreas> better readable. There were even suggestions. Also I
Andreas> implicitly suggested to use one spelling of "Freescale"
Andreas> consistently throughout your patches.

Sorry, yes I missed that comment.   V7 will come soon.


 --
Dr Peter Chubb  peter.chubb AT nicta.com.au
http://www.ssrg.nicta.com.au  Software Systems Research Group/NICTA

Re: [Qemu-devel] [PATCH] ISCSI: Add support for thin-provisioning via discard/UNMAP and bigger LUNs

[ronnie sahlberg]

On Tue, Apr 24, 2012 at 6:05 PM, Paolo Bonzini  wrote:
> Il 24/04/2012 10:02, ronnie sahlberg ha scritto:
>> So ignore this patch for now. I will redo UNMAP in the patch to
>> instead use the generic scsi function inside libiscsi.
>>
>> That will serve the purpose to verify that the public API in libiscsi
>> is sufficient for accessing the generic transport and
>> secondly that will show a useful example on how to send CDB+data to
>> and to receive data back from the generic function.
>>
>> This generic API would be what a future virt-scsi->libiscsi
>> integration would use anyway.
>
> I will be on holiday from tomorrow to May 1, and I won't be able to send
> a pull request today.  As I want to minimize the time I spend looking at
> qemu-devel, :) I'll take this patch as is, since the new libiscsi
> version is needed anyway for READ CAPACITY (16).
>
> You could write a follow-up patch to teach iscsi.c about WRITE SAME(10)
> and WRITE SAME(16) with the unmap bit set, and use generic CDB+data
> functions for those.
>

Ok. That works for me. Thanks.

regards
ronnie sahlberg

Re: [Qemu-devel] [RFC PATCH 0/9] ACPI memory hotplug

[Gleb Natapov]

On Mon, Apr 23, 2012 at 04:31:04PM +0300, Avi Kivity wrote:
> On 04/22/2012 05:20 PM, Gleb Natapov wrote:
> > On Sun, Apr 22, 2012 at 05:13:27PM +0300, Avi Kivity wrote:
> > > On 04/22/2012 05:09 PM, Gleb Natapov wrote:
> > > > On Sun, Apr 22, 2012 at 05:06:43PM +0300, Avi Kivity wrote:
> > > > > On 04/22/2012 04:56 PM, Gleb Natapov wrote:
> > > > > > start. We will need it for migration anyway.
> > > > > >
> > > > > > > hotplug-able memory slots i.e. initial system memory is not 
> > > > > > > modeled with
> > > > > > > memslots. The concept could be generalized to include all memory 
> > > > > > > though, or it
> > > > > > > could more closely follow kvm-memory slots.
> > > > > > OK, I hope final version will allow for memory < 4G to be 
> > > > > > hot-pluggable.
> > > > > 
> > > > > Why is that important?
> > > > > 
> > > > Because my feeling is that people that want to use this kind of feature
> > > > what to start using it with VMs smaller than 4G. Of course not all
> > > > memory have to be hot unpluggable. Making first 1M or event first 128M 
> > > > not
> > > > unpluggable make perfect sense.
> > > 
> > > Can't you achieve this with -m 1G, -device dimm,size=1G,populated=true
> > > -device dimm,size=1G,populated=false?
> > > 
> > From this:
> >
> > (for hw/pc.c PCI hole is currently [below_4g_mem_size, 4G), so
> > hotplugged memory should start from max(4G, above_4g_mem_size).
> >
> > I understand that hotpluggable memory can start from above 4G only. With
> > the config above we will have memory hole from 1G to PCI memory hole.
> > May be not a big problem, but I do not see technical reason for the 
> > constrain.
> >  
> > > (I don't think hotplugging below 512MB is needed, but I don't have any
> > > real data on this).
> > > 
> > 512MB looks like a reasonable limitation too, but again if there is not
> > technical reason for having the limitation why have it?
> >
> 
> I was thinking about not having tons of 128MB slots, so we don't have a
> configuration that is far from reality.  But maybe this thinking is too
> conservative.
> 
I think it is good interface to make memory that is specified with -m to
be one big unpluggable slot, but slots defined with -device should start
just above what -m specifies (after proper alignment). Memory hot-plug
granularity is controlled by slot's size parameter.

--
Gleb.

Re: [Qemu-devel] [PATCHv2 0/3] virtio: fix memory access races

[Stefan Hajnoczi]

On Mon, Apr 23, 2012 at 2:19 PM, Michael S. Tsirkin  wrote:
> This is a follow-up to my previous patch: it turns
> out that a single mb() isn't sufficient as network
> loss could still be triggered under stress.
>
> Patch 1 is repost of v1.
>
> The following two patches fix more races found
> by code inspection and comparison with vhost
> in kernel. After applying these
> patches, no more network loss was observed.
>
> Michael S. Tsirkin (3):
>  virtio: add missing mb() on notification
>  virtio: add missing mb() on enable notification
>  virtio: order index/descriptor reads
>
>  hw/virtio.c    |   11 +++
>  qemu-barrier.h |   28 +---
>  2 files changed, 36 insertions(+), 3 deletions(-)

Reviewed-by: Stefan Hajnoczi 

Re: [Qemu-devel] [PATCH] scsi refcounting fix?

[David Gibson]

On Tue, Apr 24, 2012 at 08:37:22AM +0200, Paolo Bonzini wrote:
> Il 24/04/2012 07:02, David Gibson ha scritto:
> > So the patch below fixes my assertion failure, but again, I can't say
> > I understand this well enough to be confident - it might be leaking
> > scsi reqs instead.  But if this isn't the right fix, I hope one of you
> > can help me find where the real refcounting bug is.
> 
> Thanks for the report!
> 
> The fix seems correct.  However, since refcounting is tricky, I prefer
> to follow existing patterns and make scsi_do_read look like a combination
> of scsi_*_complete + scsi_*_data.  The following does both a ref
> (like in scsi_read_data) and and an unref (like in scsi_flush_complete):
> 
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index a914611..49f5860 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -297,6 +297,13 @@ static void scsi_do_read(void *opaque, int ret)
>  }
>  }
>  
> +if (r->req.io_canceled) {
> +return;
> +}
> +
> +/* The request is used as the AIO opaque value, so add a ref.  */
> +scsi_req_ref(&r->req);
> +
>  if (r->req.sg) {
>  dma_acct_start(s->qdev.conf.bs, &r->acct, r->req.sg, BDRV_ACCT_READ);
>  r->req.resid -= r->req.sg->size;
> 
> Can you confirm that this works for you?

This seems to work for me, thanks.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [Qemu-devel] [PATCH] ISCSI: Add support for thin-provisioning via discard/UNMAP and bigger LUNs

[Paolo Bonzini]

Il 24/04/2012 10:02, ronnie sahlberg ha scritto:
> So ignore this patch for now. I will redo UNMAP in the patch to
> instead use the generic scsi function inside libiscsi.
> 
> That will serve the purpose to verify that the public API in libiscsi
> is sufficient for accessing the generic transport and
> secondly that will show a useful example on how to send CDB+data to
> and to receive data back from the generic function.
> 
> This generic API would be what a future virt-scsi->libiscsi
> integration would use anyway.

I will be on holiday from tomorrow to May 1, and I won't be able to send
a pull request today.  As I want to minimize the time I spend looking at
qemu-devel, :) I'll take this patch as is, since the new libiscsi
version is needed anyway for READ CAPACITY (16).

You could write a follow-up patch to teach iscsi.c about WRITE SAME(10)
and WRITE SAME(16) with the unmap bit set, and use generic CDB+data
functions for those.

Paolo

Re: [Qemu-devel] [PATCH] docs: fix one issue in qcow2 specs

[Kevin Wolf]

Am 24.04.2012 09:11, schrieb zwu.ker...@gmail.com:
> From: Zhi Yong Wu 
> 
> Signed-off-by: Zhi Yong Wu 
> ---
>  docs/specs/qcow2.txt |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
> index b6adcad..ae68a6e 100644
> --- a/docs/specs/qcow2.txt
> +++ b/docs/specs/qcow2.txt
> @@ -185,7 +185,7 @@ L2 table entry (for normal clusters):
>  in L2 tables that are reachable from the the active L1
>  table.
>  
> -L2 table entry (for compressed clusters; x = 62 - (cluster_size - 8)):
> +L2 table entry (for compressed clusters; x = 62 - (cluster_bits - 8)):
>  
>  Bit  0 -  x:Host cluster offset. This is usually _not_ aligned to a
>  cluster boundary!

Good catch! Thanks, applied to the block branch.

Kevin

Re: [Qemu-devel] [RFC PATCH 0/9] ACPI memory hotplug

[Gleb Natapov]

On Mon, Apr 23, 2012 at 02:31:15PM +0200, Vasilis Liaskovitis wrote:
> Hi,
> 
> On Sun, Apr 22, 2012 at 05:20:59PM +0300, Gleb Natapov wrote:
> > On Sun, Apr 22, 2012 at 05:13:27PM +0300, Avi Kivity wrote:
> > > On 04/22/2012 05:09 PM, Gleb Natapov wrote:
> > > > On Sun, Apr 22, 2012 at 05:06:43PM +0300, Avi Kivity wrote:
> > > > > On 04/22/2012 04:56 PM, Gleb Natapov wrote:
> > > > > > start. We will need it for migration anyway.
> > > > > >
> > > > > > > hotplug-able memory slots i.e. initial system memory is not 
> > > > > > > modeled with
> > > > > > > memslots. The concept could be generalized to include all memory 
> > > > > > > though, or it
> > > > > > > could more closely follow kvm-memory slots.
> > > > > > OK, I hope final version will allow for memory < 4G to be 
> > > > > > hot-pluggable.
> > > > > 
> > > > > Why is that important?
> > > > > 
> > > > Because my feeling is that people that want to use this kind of feature
> > > > what to start using it with VMs smaller than 4G. Of course not all
> > > > memory have to be hot unpluggable. Making first 1M or event first 128M 
> > > > not
> > > > unpluggable make perfect sense.
> > > 
> > > Can't you achieve this with -m 1G, -device dimm,size=1G,populated=true
> > > -device dimm,size=1G,populated=false?
> > > 
> > From this:
> > 
> > (for hw/pc.c PCI hole is currently [below_4g_mem_size, 4G), so
> > hotplugged memory should start from max(4G, above_4g_mem_size).
> > 
> > I understand that hotpluggable memory can start from above 4G only. With
> > the config above we will have memory hole from 1G to PCI memory hole.
> > May be not a big problem, but I do not see technical reason for the 
> > constrain.
> >  
> The 440fx spec mentions: "The address range from the top of main DRAM to 4
> Gbytes (top of physical memory space supported by the 440FX PCIset) is 
> normally
> mapped to PCI. The PMC forwards all accesses within this address range to 
> PCI."
> 
> What we probably want is that the initial memory map creation takes into 
> account
> all dimms specified (both populated/unpopulated) 
Yes.

> So "-m 1G, -device dimm,size=1G,populated=true -device 
> dimm,size=1G,populated=false"
> would create a system map with top of memory and start of PCI-hole at 2G. 
> 
What -m 1G means on this command line? Isn't it redundant?
May be we should make -m create non unplaggable, populated slot starting
at address 0. Ten you config above will specify 3G memory with 2G
populated (first of which is not removable) and 1G unpopulated. PCI hole
starts above 3G.

> This may require some shifting of physical address offsets around
> 3.5GB-4GB - is this the minimum PCI hole allowed?
Currently it is 1G in QEMU code.
> 
> E.g. if we specify 4x1GB DIMMs (onlt the first initially populated)   
> -m 1G, -device dimm,size=1G,populated=true -device 
> dimm,size=1G,populated=false
> -device dimm,size=1G,populated=false -device dimm,size=1G,populated=false
> 
> we create the following memory map:
> dimm0: [0,1G)
> dimm1: [1G, 2G)
> dimm2: [2G, 3G)
> dimm3: [4G, 5G) or dimm3 is split into [3G, 3.5G) and [4G, 4.5G)
> 
> does either of these options sound reasonable?
> 
We shouldn't split dimms IMO. Just unnecessary complication. Better make
bigger PCI hole.

--
Gleb.

Re: [Qemu-devel] [PATCH] ISCSI: Add support for thin-provisioning via discard/UNMAP and bigger LUNs

[Kevin Wolf]

Am 24.04.2012 08:46, schrieb Paolo Bonzini:
> Il 24/04/2012 08:29, Ronnie Sahlberg ha scritto:
>> Update the configure test for libiscsi support to detect version 1.3 or 
>> later.
>> Version 1.3 of libiscsi provides both READCAPACITY16 as well as UNMAP 
>> commands.
>>
>> Update the iscsi block layer to use READCAPACITY16 to detect the size of the 
>> LUN instead of READCAPACITY10. This allows support for LUNs larger than 2TB.
>>
>> Update to implement bdrv_aio_discard() using the UNMAP command.
>> This allows us to use thin-provisioned LUNs from TGTD and other iSCSI 
>> targets that support thin-provisioning.
> 
> Looks good.  Kevin, do you want me to take libiscsi patches via the SCSI
> tree?

Sure, if you like, go ahead. Feel free to update MAINTAINERS as well.

> As an aside, I am not really sure of the utility of adding these utility
> functions directly in libiscsi, rather than making it a pure transport
> library.  block/iscsi.c is going to grow as you add more functionality
> (e.g. WRITE SAME commands), and libiscsi will have to be updated each
> time in lockstep.
> 
> I can see the value of basic read/write/flush and readcap10/16, but with
> unmap it's starting to be a bit more specific.  Are there other clients
> of libiscsi that use these functions?  Should they be placed into
> block/iscsi.c or a new block/iscsi-cdb.c instead?

I think I agree. For the more obscure commands, the qemu driver should
probably build the CDB on its own and use a generic function.

Kevin

Re: [Qemu-devel] [PATCH] ISCSI: Add support for thin-provisioning via discard/UNMAP and bigger LUNs

[ronnie sahlberg]

On Tue, Apr 24, 2012 at 4:46 PM, Paolo Bonzini  wrote:
> Il 24/04/2012 08:29, Ronnie Sahlberg ha scritto:
>> Update the configure test for libiscsi support to detect version 1.3 or 
>> later.
>> Version 1.3 of libiscsi provides both READCAPACITY16 as well as UNMAP 
>> commands.
>>
>> Update the iscsi block layer to use READCAPACITY16 to detect the size of the 
>> LUN instead of READCAPACITY10. This allows support for LUNs larger than 2TB.
>>
>> Update to implement bdrv_aio_discard() using the UNMAP command.
>> This allows us to use thin-provisioned LUNs from TGTD and other iSCSI 
>> targets that support thin-provisioning.
>
> Looks good.  Kevin, do you want me to take libiscsi patches via the SCSI
> tree?
>
> As an aside, I am not really sure of the utility of adding these utility
> functions directly in libiscsi, rather than making it a pure transport
> library.  block/iscsi.c is going to grow as you add more functionality
> (e.g. WRITE SAME commands), and libiscsi will have to be updated each
> time in lockstep.
>
> I can see the value of basic read/write/flush and readcap10/16, but with
> unmap it's starting to be a bit more specific.  Are there other clients
> of libiscsi that use these functions?  Should they be placed into
> block/iscsi.c or a new block/iscsi-cdb.c instead?

I see your point.
I like to add scsi commands as I use them to libiscsi, since then I
dont have to
re-write the marshalling/unmarshalling code everytime in my small test
and utility programs.
For example when i want to write some one-off small tools to test something.


But, yes. There is no real need to use them directly from qemu.


So ignore this patch for now. I will redo UNMAP in the patch to
instead use the generic scsi function inside libiscsi.

That will serve the purpose to verify that the public API in libiscsi
is sufficient for accessing the generic transport and
secondly that will show a useful example on how to send CDB+data to
and to receive data back from the generic function.

This generic API would be what a future virt-scsi->libiscsi
integration would use anyway.



regards
ronnie sahlberg

Re: [Qemu-devel] [PATCH v6 0/5] FreeSCALE i.MX31 support

[Andreas Färber]

Am 23.04.2012 01:31, schrieb Peter Chubb:
> Hi all,
>Most of the files are unchanged since last time.

Indeed... On v5 I had asked you to shorten the subjects to conform to
our commit message scheme and to make patches better readable. There
were even suggestions. Also I implicitly suggested to use one spelling
of "Freescale" consistently throughout your patches.

Did you forget?

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media

[Andreas Färber]

Am 23.04.2012 18:06, schrieb Pavel Hrdina:
> Hi,
> this is the patch to fix incorrect handling of IDE floppy drive controller 
> emulation
> when no media is present. If the guest is booted without a media then the 
> drive
> was not being emulated at all but this patch enables the emulation with no 
> media present.
> 
> There was a bug in FDC emulation without media. Driver was not able to 
> recognize that
> there is no media in drive.
> 
> This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the 
> behaviour
> is as expected, i.e. as follows:
> 
> Linux guest (Fedora 16 x86_64) tries "mount /dev/fd0" and exit with error
> "mount: /dev/fd0 is not a valid block device" which is the same behavior like
> bare metal with real floppy device (you have to load floppy driver at first
> using e.g. "modprobe floppy" command).
> 
> For Windows XP guest the Windows floppy driver is trying to seek the virtual 
> drive
> when you want to open it but driver successfully detect that there is no 
> media in drive
> and then it's asking user to insert floppy media in the drive.
> 
> I also tested behavior of this patch if you start guest with "-nodefaults" 
> and both
> Windows and Linux guests detect only FDC but no drive.
> 
> Pavel
> 
> This patch has been written with help of specifications from:
> http://www.ousob.com/ng/hardware/ngd127.php
> http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
> http://wiki.osdev.org/Floppy_Disk_Controller
> 
> Signed-off-by: Pavel Hrdina 
> Signed-off-by: Michal Novotny 
> ---

Please see http://wiki.qemu.org/Contribute/SubmitAPatch for hints on
writing a commit message for a patch: Subject should have a prefix, such
as "fdc: Fix emulation for no media" and the body should not contain
personal letter-style contents such as "Hi, this is the patch". Any such
personal comments can go under the --- line where they are not committed
to git history.

Cc'ing floppy author and block maintainer.

Andreas

>  hw/fdc.c |   14 ++
>  hw/pc.c  |3 ++-
>  2 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/fdc.c b/hw/fdc.c
> index a0236b7..6791eff 100644
> --- a/hw/fdc.c
> +++ b/hw/fdc.c
> @@ -179,12 +179,14 @@ static void fd_revalidate(FDrive *drv)
>  FDriveRate rate;
>  
>  FLOPPY_DPRINTF("revalidate\n");
> -if (drv->bs != NULL && bdrv_is_inserted(drv->bs)) {
> +if (drv->bs != NULL) {
>  ro = bdrv_is_read_only(drv->bs);
>  bdrv_get_floppy_geometry_hint(drv->bs, &nb_heads, &max_track,
>&last_sect, drv->drive, &drive, &rate);
> -if (nb_heads != 0 && max_track != 0 && last_sect != 0) {
> -FLOPPY_DPRINTF("User defined disk (%d %d %d)",
> +if (!bdrv_is_inserted(drv->bs)) {
> +FLOPPY_DPRINTF("No media in drive\n");
> +} else if (nb_heads != 0 && max_track != 0 && last_sect != 0) {
> +FLOPPY_DPRINTF("User defined disk (%d %d %d)\n",
> nb_heads - 1, max_track, last_sect);
>  } else {
>  FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n", nb_heads,
> @@ -201,11 +203,12 @@ static void fd_revalidate(FDrive *drv)
>  drv->drive = drive;
>  drv->media_rate = rate;
>  } else {
> -FLOPPY_DPRINTF("No disk in drive\n");
> +FLOPPY_DPRINTF("Drive disabled\n");
>  drv->last_sect = 0;
>  drv->max_track = 0;
>  drv->flags &= ~FDISK_DBL_SIDES;
>  }
> +
>  }
>  
>  //
> @@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)
>  
>  if (!drv->bs)
>  return 0;
> +/* This is needed for driver to detect there is no media in drive */
> +if (!bdrv_is_inserted(drv->bs))
> +return 1;
>  if (drv->media_changed) {
>  drv->media_changed = 0;
>  ret = 1;
> diff --git a/hw/pc.c b/hw/pc.c
> index 1f5aacb..29a604b 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -382,7 +382,8 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t 
> above_4g_mem_size,
>  if (floppy) {
>  fdc_get_bs(fd, floppy);
>  for (i = 0; i < 2; i++) {
> -if (fd[i] && bdrv_is_inserted(fd[i])) {
> +/* If there is no media in a drive, we still have the drive 
> present */
> +if (fd[i]) {
>  bdrv_get_floppy_geometry_hint(fd[i], &nb_heads, &max_track,
>&last_sect, FDRIVE_DRV_NONE,
>&fd_type[i], &rate);


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] Fix IDE FDC emulation for no media

[Pavel Hrdina]

On 04/24/2012 08:55 AM, Paolo Bonzini wrote:

Il 23/04/2012 18:06, Pavel Hrdina ha scritto:

Hi,
this is the patch to fix incorrect handling of IDE floppy drive controller 
emulation
when no media is present. If the guest is booted without a media then the drive
was not being emulated at all but this patch enables the emulation with no 
media present.

There was a bug in FDC emulation without media. Driver was not able to 
recognize that
there is no media in drive.

This has been tested on both Fedora-16 x86_64 VM and Windows XP VM and the 
behaviour
is as expected, i.e. as follows:

Linux guest (Fedora 16 x86_64) tries "mount /dev/fd0" and exit with error
"mount: /dev/fd0 is not a valid block device" which is the same behavior like
bare metal with real floppy device (you have to load floppy driver at first
using e.g. "modprobe floppy" command).

For Windows XP guest the Windows floppy driver is trying to seek the virtual 
drive
when you want to open it but driver successfully detect that there is no media 
in drive
and then it's asking user to insert floppy media in the drive.

I also tested behavior of this patch if you start guest with "-nodefaults" and 
both
Windows and Linux guests detect only FDC but no drive.

Pavel

This patch has been written with help of specifications from:
http://www.ousob.com/ng/hardware/ngd127.php
http://www.isdaman.com/alsos/hardware/fdc/floppy.htm
http://wiki.osdev.org/Floppy_Disk_Controller

Signed-off-by: Pavel Hrdina
Signed-off-by: Michal Novotny
---
  hw/fdc.c |   14 ++
  hw/pc.c  |3 ++-
  2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index a0236b7..6791eff 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -179,12 +179,14 @@ static void fd_revalidate(FDrive *drv)
  FDriveRate rate;

  FLOPPY_DPRINTF("revalidate\n");
-if (drv->bs != NULL&&  bdrv_is_inserted(drv->bs)) {
+if (drv->bs != NULL) {
  ro = bdrv_is_read_only(drv->bs);
  bdrv_get_floppy_geometry_hint(drv->bs,&nb_heads,&max_track,
&last_sect, drv->drive,&drive,&rate);
-if (nb_heads != 0&&  max_track != 0&&  last_sect != 0) {
-FLOPPY_DPRINTF("User defined disk (%d %d %d)",
+if (!bdrv_is_inserted(drv->bs)) {
+FLOPPY_DPRINTF("No media in drive\n");
+} else if (nb_heads != 0&&  max_track != 0&&  last_sect != 0) {
+FLOPPY_DPRINTF("User defined disk (%d %d %d)\n",
 nb_heads - 1, max_track, last_sect);
  } else {
  FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n", nb_heads,
@@ -201,11 +203,12 @@ static void fd_revalidate(FDrive *drv)
  drv->drive = drive;
  drv->media_rate = rate;
  } else {
-FLOPPY_DPRINTF("No disk in drive\n");
+FLOPPY_DPRINTF("Drive disabled\n");
  drv->last_sect = 0;
  drv->max_track = 0;
  drv->flags&= ~FDISK_DBL_SIDES;
  }
+
  }

  //
@@ -937,6 +940,9 @@ static int fdctrl_media_changed(FDrive *drv)

  if (!drv->bs)
  return 0;
+/* This is needed for driver to detect there is no media in drive */
+if (!bdrv_is_inserted(drv->bs))
+return 1;
  if (drv->media_changed) {
  drv->media_changed = 0;
  ret = 1;
diff --git a/hw/pc.c b/hw/pc.c
index 1f5aacb..29a604b 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -382,7 +382,8 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t 
above_4g_mem_size,
  if (floppy) {
  fdc_get_bs(fd, floppy);
  for (i = 0; i<  2; i++) {
-if (fd[i]&&  bdrv_is_inserted(fd[i])) {
+/* If there is no media in a drive, we still have the drive 
present */
+if (fd[i]) {
  bdrv_get_floppy_geometry_hint(fd[i],&nb_heads,&max_track,
&last_sect, FDRIVE_DRV_NONE,
&fd_type[i],&rate);

Strictly speaking this final hunk should not be necessary: the fd_type
is by default FDRIVE_DRV_NONE and it is correct if there is no medium in
the drive.  However, it does not hurt.

The rest of the patch looks good.  It's strictly a bugfix and doesn't
change the hardware exposed to the guest (only the media), so
I think this does not require a compatibility property.

Reviewed-by: Paolo Bonzini

Paolo


Hi,
there is bug that you cannot see any floppy drive in guest if you use 
defaults or set up a floppy drive without media. On bare-metal you can 
see floppy drive even without media.


For qemu you can check that there is floppy drive present by using qemu 
monitor and "info block". After you inserted media to the floppy drive 
on running guest, you still cannot see any floppy drive in guest.


This patch will set floppy drive to default values if you start guest 
with floppy drive without media and after you insert proper media to 
floppy drive you can access it from 

[Qemu-devel] Fwd: buildbot failure in qemu on rhel5-default

[Gerd Hoffmann]

 Original Message 
Subject: buildbot failure in qemu on rhel5-default
Date: Mon, 23 Apr 2012 23:31:44 +0200
From: build...@spunk.home.kraxel.org
To: kra...@gmail.com

The Buildbot has detected a failed build on builder rhel5-default while
building qemu.
Full details are available at:
 http://spunk.home.kraxel.org/bb/builders/rhel5-default/builds/249

Buildbot URL: http://spunk.home.kraxel.org/bb/

Buildslave for this Build: rhel5

Build Reason: scheduler
Build Source Stamp: [branch master] 092dfc7714ad7983aeb0cada5d5983e9fde8d84c
Blamelist: Amos Kong ,Andreas Färber
,Anthony Liguori ,Blue Swirl
,David Gibson ,Dong
Xu Wang ,Eduardo Elias Ferreira
,Eric Bénard ,Kevin Wolf
,Liu Yuan ,Lluís Vilanova
,Michael Roth ,NODA, Kai
,Paolo Bonzini ,Ronnie Sahlberg
,Stefan Hajnoczi
,Stefan Weil ,Stefano
Stabellini 

BUILD FAILED: failed configure

sincerely,
 -The Buildbot


== log tail ==

Error: invalid trace backend
Please choose a supported trace backend.


== full log ==
[ Note: IPv6 connectivity needed to access this ]
http://spunk.home.kraxel.org/bb/builders/rhel5-default/builds/249/steps/configure/logs/stdio

Re: [Qemu-devel] [PATCH v6 2/5] Implement i.MX31 Clock Control Module

[Peter Chubb]

> "Paolo" == Paolo Bonzini  writes:

Paolo> Il 23/04/2012 22:54, Peter Chubb ha scritto:
Peter> What is this calculation supposed to do? It doesn't convert a
Peter> 10-bit signed twos-complement number into an int32_t, unless
Peter> I'm confused... Also, it's a rather opaque way to write "mfn &=
Peter> 0x200;".
>> 
>> I'll use a different way to calculate.  Maybe: 
>> mfn <<= (32-10); 
>> mfn >= (32-10);

Paolo> The magic that you wanted is

Paolo>mfn |= -(mfn & 0x200);


Yes.  I'd actually been thinking of mfn -= 2 * (mfn & 0x200);
but forgot the 2*.  But the shifts should be faster, and that's wjhat
I'm testing at the moment.


Peter C
--
Dr Peter Chubb  peter.chubb AT nicta.com.au
http://www.ssrg.nicta.com.au  Software Systems Research Group/NICTA

[Qemu-devel] [PATCH] docs: fix one issue in qcow2 specs

[zwu . kernel]

From: Zhi Yong Wu 

Signed-off-by: Zhi Yong Wu 
---
 docs/specs/qcow2.txt |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
index b6adcad..ae68a6e 100644
--- a/docs/specs/qcow2.txt
+++ b/docs/specs/qcow2.txt
@@ -185,7 +185,7 @@ L2 table entry (for normal clusters):
 in L2 tables that are reachable from the the active L1
 table.
 
-L2 table entry (for compressed clusters; x = 62 - (cluster_size - 8)):
+L2 table entry (for compressed clusters; x = 62 - (cluster_bits - 8)):
 
 Bit  0 -  x:Host cluster offset. This is usually _not_ aligned to a
 cluster boundary!
-- 
1.7.6

Re: [Qemu-devel] [PATCH 00/16] QEMU vhost-scsi support

[Paolo Bonzini]

Il 24/04/2012 06:21, ronnie sahlberg ha scritto:
> Hi Stefan,
> 
> A little bit off-topic but
> 
> When you design the proper place and API to plug virt-scsi into an
> external SCSI parser outside of qemu like the target in the kernel ...
> 
> It would be very nice if one could also plug virt-scsi into libiscsi
> and pass the CDBs straight to the remote iSCSI target too.
> Keep some thoughts on virt-scsi + libiscsi integration.

Yes, that makes a lot of sense.  It's a bit harder than scsi-generic but
we do want to get there.

Paolo