Re: [Qemu-devel] [PATCH] Introduce a -libvirt-caps flag as a stop-gap

2010-07-28 Thread Amit Shah
On (Tue) Jul 27 2010 [10:55:03], Anthony Liguori wrote:
> +case QEMU_OPTION_libvirt_caps:
> +printf("version: " QEMU_VERSION "\n"
> +   "package: " QEMU_PKGVERSION "\n"
> +   "caps: name,enable-kvm,no-reboot,uuid,xen-domid,drive"
> +   ",cache-v2,format,vga,serial,mem-path,chardev,balloon"
> +   ",device,rtc,netdev,sdl,topology\n");
> +exit(0);
> +break;

This can't work; people have to remember not only to update
documentation but also this case here to ensure libvirt works fine.

There are some other options that might work:
- making such a list by taking the savevm section name and version
  number for each device
- The parameters supported by devices registered with qdev (and their
  defaults)
- etc.

But basically this means libvirt will have to do more work now and also
add support for capabilities later. We can instead just keep 0.13 as it
is and move to capabilities for 0.14.

Amit



[Qemu-devel] [PATCH] block: Fix bdrv_has_zero_init

2010-07-28 Thread Kevin Wolf
Assuming that any image on a block device is not properly zero-initialized is
actually wrong: Only raw images have this problem. Any other image format
shouldn't care about it, they initialize everything properly themselves.

Signed-off-by: Kevin Wolf 
---
 block.c   |6 ++
 block/raw-posix.c |   13 +
 block/raw-win32.c |6 ++
 block/raw.c   |6 ++
 block_int.h   |7 +--
 5 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/block.c b/block.c
index be75832..98da39a 100644
--- a/block.c
+++ b/block.c
@@ -1477,10 +1477,8 @@ int bdrv_has_zero_init(BlockDriverState *bs)
 {
 assert(bs->drv);
 
-if (bs->drv->no_zero_init) {
-return 0;
-} else if (bs->file) {
-return bdrv_has_zero_init(bs->file);
+if (bs->drv->bdrv_has_zero_init) {
+return bs->drv->bdrv_has_zero_init(bs);
 }
 
 return 1;
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 8f84a37..240d84d 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -993,6 +993,11 @@ static int hdev_create(const char *filename, 
QEMUOptionParameter *options)
 return ret;
 }
 
+static int hdev_has_zero_init(BlockDriverState *bs)
+{
+return 0;
+}
+
 static BlockDriver bdrv_host_device = {
 .format_name= "host_device",
 .protocol_name= "host_device",
@@ -1002,7 +1007,7 @@ static BlockDriver bdrv_host_device = {
 .bdrv_close = raw_close,
 .bdrv_create= hdev_create,
 .create_options = raw_create_options,
-.no_zero_init   = 1,
+.bdrv_has_zero_init = hdev_has_zero_init,
 .bdrv_flush = raw_flush,
 
 .bdrv_aio_readv= raw_aio_readv,
@@ -1117,7 +1122,7 @@ static BlockDriver bdrv_host_floppy = {
 .bdrv_close = raw_close,
 .bdrv_create= hdev_create,
 .create_options = raw_create_options,
-.no_zero_init   = 1,
+.bdrv_has_zero_init = hdev_has_zero_init,
 .bdrv_flush = raw_flush,
 
 .bdrv_aio_readv = raw_aio_readv,
@@ -1222,7 +1227,7 @@ static BlockDriver bdrv_host_cdrom = {
 .bdrv_close = raw_close,
 .bdrv_create= hdev_create,
 .create_options = raw_create_options,
-.no_zero_init   = 1,
+.bdrv_has_zero_init = hdev_has_zero_init,
 .bdrv_flush = raw_flush,
 
 .bdrv_aio_readv = raw_aio_readv,
@@ -1345,7 +1350,7 @@ static BlockDriver bdrv_host_cdrom = {
 .bdrv_close = raw_close,
 .bdrv_create= hdev_create,
 .create_options = raw_create_options,
-.no_zero_init   = 1,
+.bdrv_has_zero_init = hdev_has_zero_init,
 .bdrv_flush = raw_flush,
 
 .bdrv_aio_readv = raw_aio_readv,
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 745bbde..503ed39 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -394,6 +394,11 @@ static int raw_set_locked(BlockDriverState *bs, int locked)
 }
 #endif
 
+static int hdev_has_zero_init(BlockDriverState *bs)
+{
+return 0;
+}
+
 static BlockDriver bdrv_host_device = {
 .format_name   = "host_device",
 .protocol_name = "host_device",
@@ -402,6 +407,7 @@ static BlockDriver bdrv_host_device = {
 .bdrv_file_open= hdev_open,
 .bdrv_close= raw_close,
 .bdrv_flush= raw_flush,
+.bdrv_has_zero_init = hdev_has_zero_init,
 
 .bdrv_read = raw_read,
 .bdrv_write= raw_write,
diff --git a/block/raw.c b/block/raw.c
index 1414e77..61e6748 100644
--- a/block/raw.c
+++ b/block/raw.c
@@ -237,6 +237,11 @@ static QEMUOptionParameter raw_create_options[] = {
 { NULL }
 };
 
+static int raw_has_zero_init(BlockDriverState *bs)
+{
+return bdrv_has_zero_init(bs->file);
+}
+
 static BlockDriver bdrv_raw = {
 .format_name= "raw",
 
@@ -264,6 +269,7 @@ static BlockDriver bdrv_raw = {
 
 .bdrv_create= raw_create,
 .create_options = raw_create_options,
+.bdrv_has_zero_init = raw_has_zero_init,
 };
 
 static void bdrv_raw_init(void)
diff --git a/block_int.h b/block_int.h
index f075a8c..7d5e751 100644
--- a/block_int.h
+++ b/block_int.h
@@ -127,8 +127,11 @@ struct BlockDriver {
 
 void (*bdrv_debug_event)(BlockDriverState *bs, BlkDebugEvent event);
 
-/* Set if newly created images are not guaranteed to contain only zeros */
-int no_zero_init;
+/*
+ * Returns 1 if newly created images are guaranteed to contain only
+ * zeros, 0 otherwise.
+ */
+int (*bdrv_has_zero_init)(BlockDriverState *bs);
 
 QLIST_ENTRY(BlockDriver) list;
 };
-- 
1.7.1.1




[Qemu-devel] [PATCH] Use kvm32/kvm64 as default CPUs when running under KVM.

2010-07-28 Thread Jes . Sorensen
From: Jes Sorensen 

KVM has a minimum CPU requirement in order to run, so there is no
reason to default to the very basic family 6, model 2 (or model 3 for
qemu32) CPU since the additional features are going to be available on
the host CPU.

Signed-off-by: Jes Sorensen 
---
 hw/pc.c |   13 +++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 58dea57..b17a199 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -39,6 +39,7 @@
 #include "msix.h"
 #include "sysbus.h"
 #include "sysemu.h"
+#include "kvm.h"
 
 /* output Bochs bios info messages */
 //#define DEBUG_BIOS
@@ -866,11 +867,19 @@ void pc_cpus_init(const char *cpu_model)
 
 /* init CPUs */
 if (cpu_model == NULL) {
+if (kvm_enabled()) {
 #ifdef TARGET_X86_64
-cpu_model = "qemu64";
+cpu_model = "kvm64";
 #else
-cpu_model = "qemu32";
+cpu_model = "kvm32";
 #endif
+} else {
+#ifdef TARGET_X86_64
+cpu_model = "qemu64";
+#else
+cpu_model = "qemu32";
+#endif
+}
 }
 
 for(i = 0; i < smp_cpus; i++) {
-- 
1.7.1.1




Re: [Qemu-devel] Re: [PATCH] Introduce a -libvirt-caps flag as a stop-gap

2010-07-28 Thread Daniel P. Berrange
On Tue, Jul 27, 2010 at 12:20:55PM -0500, Anthony Liguori wrote:
> On 07/27/2010 12:00 PM, Daniel P. Berrange wrote:
> >>Yup.  You'll need to decide up front if you want to probe for a feature
> >>when it's introduced and have something added to capabilities.
> >>
> >>This is simple though.  A few weeks before 0.14 is released, go through
> >>the change log, and anything that looks interesting, add a cap flag.
> >> 
> >That doesn't work for features which already exist in QEMU which are
> >not yet supported in libvirt. eg consider QEMU 0.13 is released, and
> >then we want to add a new feature to libvirt a month later.
> 
> Right.  So sit down and look at the 0.13 changelog and if there's any 
> features that you think you might want to support at some point in time, 
> add a capability.

Not really practical - pretty much anything is a candidate because
we want to support as many of QEMU features as possible.

> >It offers significantly less information that the existing -help
> >data, so I don't think it is workable as a replacement. We'd get
> >into the bad situation where we could support a feature in 0.12
> >but not in 0.13, unless we went back to using -help output again.
> >
> >If we're going for a short term hack, then how about a combination
> >of
> >
> >http://www.mail-archive.com/qemu-devel@nongnu.org/msg34944.html
> >http://www.mail-archive.com/qemu-devel@nongnu.org/msg34960.html
> >   
> 
> Would have failed in exactly the same way that the current -help parsing 
> fails.  The description of an argument in the help text is not a 
> capabilities string.

This particular case of cache modes might have failed, but in the
broader picture it would have been more reliable. The vasty 
majority of the time, we only check whether a particular named
argument exists. This patch would make it very reliable todo so
because you could simply extract a single named field from the
JSON doc. The cases where we looked at the parameter options 
would have been improved a little, but still be potentially fragile.
Checking for version number would also be improved. So overall 
while obviously not perfect, it would be significantly better than
the solution today and using an approach that isn't a complete
throwaway solution.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|



Re: [Qemu-devel] [RFC PATCH 02/14] KVM Test: Add a function get_interface_name() to kvm_net_utils.py

2010-07-28 Thread Michael Goldish
On 07/27/2010 05:08 AM, Lucas Meneghel Rodrigues wrote:
> On Tue, 2010-07-20 at 09:35 +0800, Amos Kong wrote:
>> The function get_interface_name is used to get the interface name of linux
>> guest through the macaddress of specified macaddress.
> 
> I wonder if it wouldn't be overkill to have separate utility libraries
> on the kvm test instead of a single kvm_utils and kvm_test_utils like
> you are proposing. Any thoughts Michael?

Yes, looks like this could be in kvm_test_utils.py, especially if
there's only a small number of functions here.

>> Signed-off-by: Jason Wang 
>> Signed-off-by: Amos Kong 
>> ---
>>  0 files changed, 0 insertions(+), 0 deletions(-)
>>
>> diff --git a/client/tests/kvm/kvm_net_utils.py 
>> b/client/tests/kvm/kvm_net_utils.py
>> new file mode 100644
>> index 000..ede4965
>> --- /dev/null
>> +++ b/client/tests/kvm/kvm_net_utils.py
>> @@ -0,0 +1,18 @@
>> +import re
>> +
>> +def get_linux_ifname(session, mac_address):
>> +"""
>> +Get the interface name through the mac address.
>> +
>> +@param session: session to the virtual machine
>> +@mac_address: the macaddress of nic
>> +"""
>> +
>> +output = session.get_command_output("ifconfig -a")
>> +
>> +try:
>> +ethname = re.findall("(\w+)\s+Link.*%s" % mac_address, output,
>> + re.IGNORECASE)[0]
>> +return ethname
>> +except:
>> +return None
>>
>>
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Qemu-devel] Re: [PATCH] vmstate: fix vmstate_subsection_load

2010-07-28 Thread Juan Quintela
TeLeMan  wrote:
>  If the new version adds the new subsection for some vmstate, the old
> version will load the new version's vmstate unsuccessfully. So we have
> to ignore the unrecognized subsections.

No.  That was the whole point of subsections.  If one subsection is
sent, target machine has to understand it.  If it don't understand it,
it fails.

If subsection is not needed, it is the responsability of the source to
not send it.

This was one of the design requirements.  Subsections are optional but
it is the source which decides which ones to send/not to send.  The
target has to understand everything that it gets or fail the migration.

Later, Juan.

> Signed-off-by: TeLeMan 
> ---
>  savevm.c |   11 +--
>  1 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/savevm.c b/savevm.c
> index 9a8328d..3e1aa73 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -1581,12 +1581,11 @@ static int vmstate_subsection_load(QEMUFile
> *f, const VMStateDescription *vmsd,
>  version_id = qemu_get_be32(f);
>
>  sub_vmsd = vmstate_get_subsection(vmsd->subsections, idstr);
> -if (sub_vmsd == NULL) {
> -return -ENOENT;
> -}
> -ret = vmstate_load_state(f, sub_vmsd, opaque, version_id);
> -if (ret) {
> -return ret;
> +if (sub_vmsd) {
> +ret = vmstate_load_state(f, sub_vmsd, opaque, version_id);
> +if (ret) {
> +return ret;
> +}
>  }
>  }
>  return 0;



Re: [Qemu-devel] [PATCH] Use kvm32/kvm64 as default CPUs when running under KVM.

2010-07-28 Thread Avi Kivity

 On 07/28/2010 01:05 PM, jes.soren...@redhat.com wrote:

From: Jes Sorensen

KVM has a minimum CPU requirement in order to run, so there is no
reason to default to the very basic family 6, model 2 (or model 3 for
qemu32) CPU since the additional features are going to be available on
the host CPU.


@@ -866,11 +867,19 @@ void pc_cpus_init(const char *cpu_model)

  /* init CPUs */
  if (cpu_model == NULL) {
+if (kvm_enabled()) {
  #ifdef TARGET_X86_64
-cpu_model = "qemu64";
+cpu_model = "kvm64";
  #else
-cpu_model = "qemu32";
+cpu_model = "kvm32";
  #endif
+} else {
+#ifdef TARGET_X86_64
+cpu_model = "qemu64";
+#else
+cpu_model = "qemu32";
+#endif
+}
  }


What about -M 0.12?  It needs to retain the old values.

--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.




[Qemu-devel] Re: [PATCH] block: Change bdrv_eject() not to drop the image

2010-07-28 Thread Kevin Wolf
Am 27.07.2010 14:02, schrieb Markus Armbruster:
> bdrv_eject() gets called when a device model opens or closes the tray.
> 
> If the block driver implements method bdrv_eject(), that method gets
> called.  Drivers host_cdrom implements it, and it opens and closes the
> physical tray, and nothing else.  When a device model opens, then
> closes the tray, media changes only if the user actively changes the
> physical media while the tray is open.  This is matches how physical
> hardware behaves.
> 
> If the block driver doesn't implement method bdrv_eject(), we do
> something quite different: opening the tray severs the connection to
> the image by calling bdrv_close(), and closing the tray does nothing.
> When the device model opens, then closes the tray, media is gone,
> unless the user actively inserts another one while the tray is open,
> with a suitable change command in the monitor.  This isn't how
> physical hardware behaves.  Rather inconvenient when programs
> "helpfully" eject media to give you a chance to change it.  The way
> bdrv_eject() behaves here turns that chance into a must, which is not
> what these programs or their users expect.
> 
> Change the default action not to call bdrv_close().  Instead, note the
> tray status in new BlockDriverState member tray_open.  Use it in
> bdrv_is_inserted().
> 
> Arguably, the device models should keep track of tray status
> themselves.  But this is less invasive.
> 
> Signed-off-by: Markus Armbruster 

Thanks, applied to the block branch.

Kevin



Re: [Qemu-devel] [PATCH] Use kvm32/kvm64 as default CPUs when running under KVM.

2010-07-28 Thread Jes Sorensen
On 07/28/10 12:51, Avi Kivity wrote:
>  On 07/28/2010 01:05 PM, jes.soren...@redhat.com wrote:
>> From: Jes Sorensen
>>
>> KVM has a minimum CPU requirement in order to run, so there is no
>> reason to default to the very basic family 6, model 2 (or model 3 for
>> qemu32) CPU since the additional features are going to be available on
>> the host CPU.
>>
>>
>> @@ -866,11 +867,19 @@ void pc_cpus_init(const char *cpu_model)
>>
>>   /* init CPUs */
>>   if (cpu_model == NULL) {
>> +if (kvm_enabled()) {
>>   #ifdef TARGET_X86_64
>> -cpu_model = "qemu64";
>> +cpu_model = "kvm64";
>>   #else
>> -cpu_model = "qemu32";
>> +cpu_model = "kvm32";
>>   #endif
>> +} else {
>> +#ifdef TARGET_X86_64
>> +cpu_model = "qemu64";
>> +#else
>> +cpu_model = "qemu32";
>> +#endif
>> +}
>>   }
> 
> What about -M 0.12?  It needs to retain the old values.
> 

Urgh, good point. I'll come up with a fix for that. So much for a simple
patch :)

Jes




Re: [Qemu-devel] Re: KVM call agenda for July 27

2010-07-28 Thread Markus Armbruster
Anthony Liguori  writes:

> On 07/27/2010 10:22 AM, Markus Armbruster wrote:
>> Kevin Wolf  writes:
>>
>>
>>> Am 27.07.2010 15:00, schrieb Anthony Liguori:
>>>  
 On 07/27/2010 02:19 AM, Markus Armbruster wrote:

> Anthony Liguori   writes:
>
>
>  
>> - any additional input on probed_raw?
>>
>>
> Isn't it a fait accompli?  I stopped providing input when commit
> 79368c81 appeared.
>
>  
 No.  79368c81 was to close the security hole (and I do consider it a
 security hole).  But as I mentioned on the list, I'm also not satisfied
 with it and that's why I proposed probed_raw.  I was hoping to get a
 little more input from those that objected to 79368c81 as to whether
 probed_raw was more agreeable.

>>> Actually I believe qraw is less agreeable. It just too much magic. You
>>> wouldn't expect that your raw images are turned into some other format
>>> that you can't mount or use with any other program any more.
>>>  
>> I also dislike probed_raw, for the same reasons.
>>
>> Raw can't be probed safely, by its very nature.  For historical reasons,
>> we try anyway.  I think we should stop doing that, even though that
>> breaks existing use relying on the misfeature.  Announce it now, spit
>> out scary warnings, kill it for good 1-2 releases later.
>>
>> If we're unwilling to do that, then I'd *strongly* prefer doing nothing
>> over silently messing with the raw writes to sector 0 (so does
>> Christoph, and he explained why).
>
> If we add docs/deprecated-features.txt, schedule removal for at least
> 1 year in the future, and put a warning in the code that prints
> whenever raw is probed, I think I could warm up to this.
>
> Since libvirt should be insulating users from this today, I think the
> fall out might not be terrible.

Okay, I'll prepare a patch.



[Qemu-devel] Re: [PATCH] vmstate: fix vmstate_subsection_load

2010-07-28 Thread TeLeMan
On Wed, Jul 28, 2010 at 18:43, Juan Quintela  wrote:
> TeLeMan  wrote:
>>  If the new version adds the new subsection for some vmstate, the old
>> version will load the new version's vmstate unsuccessfully. So we have
>> to ignore the unrecognized subsections.
>
> No.  That was the whole point of subsections.  If one subsection is
> sent, target machine has to understand it.  If it don't understand it,
> it fails.
>
> If subsection is not needed, it is the responsability of the source to
> not send it.
>
> This was one of the design requirements.  Subsections are optional but
> it is the source which decides which ones to send/not to send.  The
> target has to understand everything that it gets or fail the migration.
>

If the target must understand everything, the vmstate's version will
be useless because the old version target maybe cannot load the new
version target's vmstate.



Re: [Qemu-devel] Re: KVM call agenda for July 27

2010-07-28 Thread Kevin Wolf
Am 28.07.2010 13:22, schrieb Markus Armbruster:
> Anthony Liguori  writes:
> 
>> On 07/27/2010 10:22 AM, Markus Armbruster wrote:
>>> Kevin Wolf  writes:
>>>
>>>
 Am 27.07.2010 15:00, schrieb Anthony Liguori:
  
> On 07/27/2010 02:19 AM, Markus Armbruster wrote:
>
>> Anthony Liguori   writes:
>>
>>
>>  
>>> - any additional input on probed_raw?
>>>
>>>
>> Isn't it a fait accompli?  I stopped providing input when commit
>> 79368c81 appeared.
>>
>>  
> No.  79368c81 was to close the security hole (and I do consider it a
> security hole).  But as I mentioned on the list, I'm also not satisfied
> with it and that's why I proposed probed_raw.  I was hoping to get a
> little more input from those that objected to 79368c81 as to whether
> probed_raw was more agreeable.
>
 Actually I believe qraw is less agreeable. It just too much magic. You
 wouldn't expect that your raw images are turned into some other format
 that you can't mount or use with any other program any more.
  
>>> I also dislike probed_raw, for the same reasons.
>>>
>>> Raw can't be probed safely, by its very nature.  For historical reasons,
>>> we try anyway.  I think we should stop doing that, even though that
>>> breaks existing use relying on the misfeature.  Announce it now, spit
>>> out scary warnings, kill it for good 1-2 releases later.
>>>
>>> If we're unwilling to do that, then I'd *strongly* prefer doing nothing
>>> over silently messing with the raw writes to sector 0 (so does
>>> Christoph, and he explained why).
>>
>> If we add docs/deprecated-features.txt, schedule removal for at least
>> 1 year in the future, and put a warning in the code that prints
>> whenever raw is probed, I think I could warm up to this.
>>
>> Since libvirt should be insulating users from this today, I think the
>> fall out might not be terrible.
> 
> Okay, I'll prepare a patch.

This kills -hda and friends for raw images. I'm not sure this is a good
idea.

Kevin



[Qemu-devel] Re: [PATCH] Ignore writes of perf reg (cp15 with crm == 12)

2010-07-28 Thread Arnd Bergmann
On Sunday 25 July 2010, Loïc Minier wrote:
> On ARMv7, ignore writes to cp15 with crm == 12; these are to setup perf
> counters which we don't have.

Thanks Loïc, I have tested this and can confirm that I'm able to boot
a CONFIG_PERF_EVENTS kernel now.

Arnd



Re: [Qemu-devel] [RFC PATCH 03/14] KVM Test: Add a common ping module for network related tests

2010-07-28 Thread Michael Goldish
On 07/27/2010 04:01 PM, Lucas Meneghel Rodrigues wrote:
> On Tue, 2010-07-20 at 09:35 +0800, Amos Kong wrote:
>> The kvm_net_utils.py is a just a place that wraps common network
>> related commands which is used to do the network-related tests.
>> Use -1 as the packet ratio for loss analysis.
>> Use quiet mode when doing the flood ping.
>>
>> Signed-off-by: Jason Wang 
>> Signed-off-by: Amos Kong 
>> ---
>>  0 files changed, 0 insertions(+), 0 deletions(-)
>>
>> diff --git a/client/tests/kvm/kvm_net_utils.py 
>> b/client/tests/kvm/kvm_net_utils.py
>> index ede4965..8a71858 100644
>> --- a/client/tests/kvm/kvm_net_utils.py
>> +++ b/client/tests/kvm/kvm_net_utils.py
>> @@ -1,4 +1,114 @@
>> -import re
>> +import logging, re, signal
>> +from autotest_lib.client.common_lib import error
>> +import kvm_subprocess, kvm_utils
>> +
>> +def get_loss_ratio(output):
>> +"""
>> +Get the packet loss ratio from the output of ping
>> +
>> +@param output
>> +"""
>> +try:
>> +return int(re.findall('(\d+)% packet loss', output)[0])
>> +except IndexError:
>> +logging.debug(output)
>> +return -1
> 
>> +def raw_ping(command, timeout, session, output_func):
>> +"""
>> +Low-level ping command execution.
>> +
>> +@param command: ping command
>> +@param timeout: timeout of the ping command
>> +@param session: local executon hint or session to execute the ping 
>> command
>> +"""
>> +if session == "localhost":

I have no problem with this, but wouldn't it be nicer to use None to
indicate that the session should be local?

>> +process = kvm_subprocess.run_bg(command, output_func=output_func,
>> +timeout=timeout)
> 
> Do we really have to resort to kvm_subprocess here? We use subprocess on
> special occasions, usually when the command in question is required to
> live throughout the tests, which doesn't seem to be the case.
> kvm_subprocess is a good library, but it is a more heavyweight
> alternative (spawns its own shell process, for example). Maybe you are
> using it because you can directly send input to the process at any time,
> such as the ctrl+c later on this same code.
> 
>> +
>> +# Send SIGINT singal to notify the timeout of running ping process,
> 
> ^ typo, signal
> 
>> +# Because ping have the ability to catch the SIGINT signal so we can
>> +# always get the packet loss ratio even if timeout.
>> +if process.is_alive():
>> +kvm_utils.kill_process_tree(process.get_pid(), signal.SIGINT)
>> +
>> +status = process.get_status()
>> +output = process.get_output()
>> +
>> +process.close()
>> +return status, output
>> +else:
>> +session.sendline(command)
>> +status, output = session.read_up_to_prompt(timeout=timeout,
>> +   print_func=output_func)
>> +if status is False:
> 
> ^ For None objects, it is better to explicitly test for is None. However
> the above construct seems more natural if put as
> 
> if not status:
> 
> Any particular reason you tested explicitely for False?

read_up_to_prompt() returns True/False as the first member of the tuple.

>> +# Send ctrl+c (SIGINT) through ssh session
>> +session.sendline("\003")

I think you can use send() here.  sendline() calls send() with a newline
added to the string.

>> +status, output2 = 
>> session.read_up_to_prompt(print_func=output_func)
>> +output += output2
>> +if status is False:
>> +# We also need to use this session to query the return value
>> +session.sendline("\003")

Same here.

>> +
>> +session.sendline(session.status_test_command)
>> +s2, o2 = session.read_up_to_prompt()
>> +if s2 is False:
> 
> ^ See comment above
> 
>> +status = -1
>> +else:
>> +try:
>> +status = int(re.findall("\d+", o2)[0])
>> +except:
>> +status = -1
>> +
>> +return status, output
>> +
>> +def ping(dest = "localhost", count = None, interval = None, interface = 
>> None,
>> + packetsize = None, ttl = None, hint = None, adaptive = False,
>> + broadcast = False, flood = False, timeout = 0,
>> + output_func = logging.debug, session = "localhost"):
> 
> ^ On function headers, we pretty much follow PEP 8 and don't use spacing
> between argument keys, the equal sign and the default value, so this
> header should be rewritten
> 
> def ping(dest="localhost",...)
> 
>> +"""
>> +Wrapper of ping.
>> +
>> +@param dest: destination address
>> +@param count: count of icmp packet
>> +@param interval: interval of two icmp echo request
>> +@param interface: specified interface of the source address
>> +@param packetsize: packet size of icmp
>> +@param ttl: ip time to live
>> +@param hin

[Qemu-devel] Re: [PATCH] vmstate: fix vmstate_subsection_load

2010-07-28 Thread Juan Quintela
TeLeMan  wrote:
> On Wed, Jul 28, 2010 at 18:43, Juan Quintela  wrote:
>> TeLeMan  wrote:
>>>  If the new version adds the new subsection for some vmstate, the old
>>> version will load the new version's vmstate unsuccessfully. So we have
>>> to ignore the unrecognized subsections.
>>
>> No.  That was the whole point of subsections.  If one subsection is
>> sent, target machine has to understand it.  If it don't understand it,
>> it fails.
>>
>> If subsection is not needed, it is the responsability of the source to
>> not send it.
>>
>> This was one of the design requirements.  Subsections are optional but
>> it is the source which decides which ones to send/not to send.  The
>> target has to understand everything that it gets or fail the migration.
>>
>
> If the target must understand everything, the vmstate's version will
> be useless because the old version target maybe cannot load the new
> version target's vmstate.

That is the whole point.  See the ide example that I also sent.

We have an old version of qemu, that don't understand pio ide fields.
We get a new qemu version that has pio ide fields in the state.

notice that migration in the middle of a pio operation is a very rare
event.  for instance, if you are using linux as a guest, pio is only
used during boot, and not so many times (around 3 calls during
boot).

Without subsections, you will be unable to migrate from new qemu to old
qemu, because old qemu don't understand the pio fields.

With subsections, if we are in the middle of a pio operation, we sent
pio subsection and migration to old qemu will fail.

But if we aren't in the middle of a pio operation, pio information will
not be sent and migration will succeed.

This is special important for the stable branch, where we are supposed
to be backwards compatible, but sometimes we find a bug, and we really
have to change the savevm format.

This allows us that after creating the subsection, we can still migrate
to the old version the majority of the time.

Notice that this bugfixes are normally rare cases, because if it were
the normal case, we would have already detected it before the release.

If we sent the subsection, it means that target needs to understand it,
or state will be broken (one way or another).

I hope this hepls to understand how subsections are supposed to work.

Thanks for the comments, Juan.



[Qemu-devel] Présentation SUPERONLINE

2010-07-28 Thread cbouy89en95superonline

 
Madame, Monsieur Bonjour, 
 
Ne jetez-pas ce mail à la corbeille sans en avoir pris connaîssance.
Que vous soyez chez vous ou au bureau !!!
 
Connaîssez-vous quelqu'un qui mange tout les jours ?
Tout le monde bien sur !!!
Connaîssez-vous un supermarché qui vous paye pour faire vos courses ? 
Non !!!
Moi oui SUPERONLINE !!!  
 
Vous recevez ce mail d'information qui vous présente cette opportunité 
novatrice et révolutionnaire de job avec un très gros avantage, peu de temps à 
y consacrer, Gain à la hauteur de vos ambitions, statut VDI-VDR.
 
Ou simplement pour devenir consommateur adhérent si vous le souhaitez.
Vos obligations en parler à 5 personnes que vous connaîssez c'est tout.
SUPERONLINE c'est un supermarché en ligne qui est aligné sur des prix de marché 
espagnol donc plus bas que nos supermarchés Français et en plus on vous paye 
pour faire du bouche à oreille.
 
C'est simple et attrayant.
 
Cordiales salutations.
 
Demandez-nous de l'information par mail en retour.
Pour faire partie de cette formidable aventure vous devez-être parrainné.
Contactez-nous !
 


Christian BOUY
Créateur de réseau chez 
SUPERONLINE
 
95800 Cergy le haut
Contact Mail : cbouy89en95superonl...@orange.fr
Skype : Christian89en95
Site Web : http://pagesperso-orange.fr/SUPERONLINE-CB/
  
  Tel :   06 07 44 78 49
09 77 32 85 66
 
Conformément à la loi sur les libertés informatiques, vous disposez d'un droit 
de retrait de votre adresse mail sur ce listing sur simple demande par mail à 
cette même adresse ci-dessus.


[Qemu-devel] Re: [PATCH] vmstate: fix vmstate_subsection_load

2010-07-28 Thread TeLeMan
On Wed, Jul 28, 2010 at 19:51, Juan Quintela  wrote:
> TeLeMan  wrote:
>> On Wed, Jul 28, 2010 at 18:43, Juan Quintela  wrote:
>>> TeLeMan  wrote:
  If the new version adds the new subsection for some vmstate, the old
 version will load the new version's vmstate unsuccessfully. So we have
 to ignore the unrecognized subsections.
>>>
>>> No.  That was the whole point of subsections.  If one subsection is
>>> sent, target machine has to understand it.  If it don't understand it,
>>> it fails.
>>>
>>> If subsection is not needed, it is the responsability of the source to
>>> not send it.
>>>
>>> This was one of the design requirements.  Subsections are optional but
>>> it is the source which decides which ones to send/not to send.  The
>>> target has to understand everything that it gets or fail the migration.
>>>
>>
>> If the target must understand everything, the vmstate's version will
>> be useless because the old version target maybe cannot load the new
>> version target's vmstate.
>
> That is the whole point.  See the ide example that I also sent.
>
> We have an old version of qemu, that don't understand pio ide fields.
> We get a new qemu version that has pio ide fields in the state.
>
> notice that migration in the middle of a pio operation is a very rare
> event.  for instance, if you are using linux as a guest, pio is only
> used during boot, and not so many times (around 3 calls during
> boot).
>
> Without subsections, you will be unable to migrate from new qemu to old
> qemu, because old qemu don't understand the pio fields.
>
> With subsections, if we are in the middle of a pio operation, we sent
> pio subsection and migration to old qemu will fail.
>
> But if we aren't in the middle of a pio operation, pio information will
> not be sent and migration will succeed.
>
> This is special important for the stable branch, where we are supposed
> to be backwards compatible, but sometimes we find a bug, and we really
> have to change the savevm format.
>
> This allows us that after creating the subsection, we can still migrate
> to the old version the majority of the time.
>
> Notice that this bugfixes are normally rare cases, because if it were
> the normal case, we would have already detected it before the release.
>
> If we sent the subsection, it means that target needs to understand it,
> or state will be broken (one way or another).
>
> I hope this hepls to understand how subsections are supposed to work.
>
> Thanks for the comments, Juan.
>
I see, thanks a lot. But I still hope to have the similar subsection
that can be ignored simply.



[Qemu-devel] [Bug 595438] Re: KVM segmentation fault, using SCSI+writeback and linux 2.4 guest

2010-07-28 Thread Коренберг Марк
Path has been applied to upstream
http://thread.gmane.org/gmane.comp.emulators.qemu/76376


** Changed in: qemu
   Status: Confirmed => Fix Committed

-- 
KVM segmentation fault, using SCSI+writeback and linux 2.4 guest
https://bugs.launchpad.net/bugs/595438
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.

Status in QEMU: New
Status in qemu-kvm: Fix Committed

Bug description:
I Use Ubuntu 32 bit 10.04 with standard KVM.
I have Intel E7600  @ 3.06GHz processor with VMX

In this system I Run:
LC_ALL=C PATH=/usr/local/sbin:/usr/local/bin:/usr/bin:/usr/sbin:/sbin:/bin 
QEMU_AUDIO_DRV=none /usr/bin/kvm -M pc-0.12 -enable-kvm -m 256 -smp 1 -name 
spamsender -uuid b9cacd5e-08f7-41fd-78c8-89cec59af881 -chardev 
socket,id=monitor,path=/var/lib/libvirt/qemu/spamsender.monitor,server,nowait 
-monitor chardev:monitor -boot d -drive 
file=/mnt/megadiff/cdiso_400_130.iso,if=ide,media=cdrom,index=2 -drive 
file=/home/mmarkk/spamsender2.img,if=scsi,index=0,format=qcow2,cache=writeback 
-net nic,macaddr=00:00:00:00:00:00,vlan=0,name=nic.0 -net tap,vlan=0,name=tap.0 
-chardev pty,id=serial0 -serial chardev:serial0 -parallel none -usb -vnc 
127.0.0.1:0 -vga cirrus

.iso image contain custom distro of 2.4-linux kernel based system. During 
install process (when .tar.gz actively unpacked), kvm dead with segmentation 
fault.

And ONLY when I choose scsi virtual disk and writeback simultaneously. 
But, writeback+ide, writethrough+scsi works OK.

I use qcow2. It seems, that qcow does not have such problems.

Virtual machine get down at random time during file copy. It seems, when qcow2 
file size need to be expanded.







[Qemu-devel] Re: [PATCH] vmstate: fix vmstate_subsection_load

2010-07-28 Thread Juan Quintela
TeLeMan  wrote:
> On Wed, Jul 28, 2010 at 19:51, Juan Quintela  wrote:
>> I hope this hepls to understand how subsections are supposed to work.
>>
>> Thanks for the comments, Juan.
>>
> I see, thanks a lot. But I still hope to have the similar subsection
> that can be ignored simply.

Then it is better not to be sent in the 1st place.

Do you have any example of why you want to do?  When I dessigned
subsections, I looked at all the changes that we had from 0.11 to 0.12,
subsections can handle all of them.  Just curious about what you need.\

Notice that ignoring subsections at this point is not trivial, as
(sub)sections don't have a size field.  Working on getting size there,
but it is a long term project (it requires 1st to change everything to
VMState to be able to change how QEMUFile works).

Later, Juan.



[Qemu-devel] [Bug 595438] Re: KVM segmentation fault, using SCSI+writeback and linux 2.4 guest

2010-07-28 Thread Коренберг Марк
** Project changed: qemu => qemu-kvm

** Also affects: qemu
   Importance: Undecided
   Status: New

** Also affects: kvm
   Importance: Undecided
   Status: New

** Changed in: qemu
   Status: New => Fix Committed

** Changed in: qemu-kvm
   Status: Fix Committed => Confirmed

** Changed in: kvm
   Status: New => Confirmed

-- 
KVM segmentation fault, using SCSI+writeback and linux 2.4 guest
https://bugs.launchpad.net/bugs/595438
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.

Status in Kernel Virtual Machine: Confirmed
Status in QEMU: Fix Committed
Status in qemu-kvm: Confirmed

Bug description:
I Use Ubuntu 32 bit 10.04 with standard KVM.
I have Intel E7600  @ 3.06GHz processor with VMX

In this system I Run:
LC_ALL=C PATH=/usr/local/sbin:/usr/local/bin:/usr/bin:/usr/sbin:/sbin:/bin 
QEMU_AUDIO_DRV=none /usr/bin/kvm -M pc-0.12 -enable-kvm -m 256 -smp 1 -name 
spamsender -uuid b9cacd5e-08f7-41fd-78c8-89cec59af881 -chardev 
socket,id=monitor,path=/var/lib/libvirt/qemu/spamsender.monitor,server,nowait 
-monitor chardev:monitor -boot d -drive 
file=/mnt/megadiff/cdiso_400_130.iso,if=ide,media=cdrom,index=2 -drive 
file=/home/mmarkk/spamsender2.img,if=scsi,index=0,format=qcow2,cache=writeback 
-net nic,macaddr=00:00:00:00:00:00,vlan=0,name=nic.0 -net tap,vlan=0,name=tap.0 
-chardev pty,id=serial0 -serial chardev:serial0 -parallel none -usb -vnc 
127.0.0.1:0 -vga cirrus

.iso image contain custom distro of 2.4-linux kernel based system. During 
install process (when .tar.gz actively unpacked), kvm dead with segmentation 
fault.

And ONLY when I choose scsi virtual disk and writeback simultaneously. 
But, writeback+ide, writethrough+scsi works OK.

I use qcow2. It seems, that qcow does not have such problems.

Virtual machine get down at random time during file copy. It seems, when qcow2 
file size need to be expanded.







[Qemu-devel] [PATCH 1/2] Set a default CPU type for compat PC machine defs.

2010-07-28 Thread Jes . Sorensen
From: Jes Sorensen 

This allows for changing the default CPU type on the current PC
definition without breaking legacy mode.

Signed-off-by: Jes Sorensen 
---
 hw/boards.h  |1 +
 hw/pc_piix.c |   15 +++
 vl.c |2 ++
 3 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/hw/boards.h b/hw/boards.h
index 6f0f0d7..932bc23 100644
--- a/hw/boards.h
+++ b/hw/boards.h
@@ -19,6 +19,7 @@ typedef struct QEMUMachine {
 QEMUMachineInitFunc *init;
 int use_scsi;
 int max_cpus;
+const char *def_cpu_model;
 unsigned int no_serial:1,
 no_parallel:1,
 use_virtcon:1,
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 812ddfd..51742a0 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -223,6 +223,11 @@ static QEMUMachine pc_machine_v0_12 = {
 .desc = "Standard PC",
 .init = pc_init_pci,
 .max_cpus = 255,
+#ifdef TARGET_X86_64
+.def_cpu_model = "qemu64",
+#else
+.def_cpu_model = "qemu32",
+#endif
 .compat_props = (GlobalProperty[]) {
 {
 .driver   = "virtio-serial-pci",
@@ -242,6 +247,11 @@ static QEMUMachine pc_machine_v0_11 = {
 .desc = "Standard PC, qemu 0.11",
 .init = pc_init_pci,
 .max_cpus = 255,
+#ifdef TARGET_X86_64
+.def_cpu_model = "qemu64",
+#else
+.def_cpu_model = "qemu32",
+#endif
 .compat_props = (GlobalProperty[]) {
 {
 .driver   = "virtio-blk-pci",
@@ -277,6 +287,11 @@ static QEMUMachine pc_machine_v0_10 = {
 .desc = "Standard PC, qemu 0.10",
 .init = pc_init_pci,
 .max_cpus = 255,
+#ifdef TARGET_X86_64
+.def_cpu_model = "qemu64",
+#else
+.def_cpu_model = "qemu32",
+#endif
 .compat_props = (GlobalProperty[]) {
 {
 .driver   = "virtio-blk-pci",
diff --git a/vl.c b/vl.c
index ba6ee11..ca2c509 100644
--- a/vl.c
+++ b/vl.c
@@ -2868,6 +2868,8 @@ int main(int argc, char **argv, char **envp)
 }
 qemu_add_globals();
 
+if ((cpu_model == NULL) && (machine->def_cpu_model))
+cpu_model = machine->def_cpu_model;
 machine->init(ram_size, boot_devices,
   kernel_filename, kernel_cmdline, initrd_filename, cpu_model);
 
-- 
1.7.1.1




[Qemu-devel] [PATCH 2/2] Use kvm32/kvm64 as default CPUs when running under KVM.

2010-07-28 Thread Jes . Sorensen
From: Jes Sorensen 

KVM has a minimum CPU requirement in order to run, so there is no
reason to default to the very basic family 6, model 2 (or model 3 for
qemu32) CPU since the additional features are going to be available on
the host CPU.

Signed-off-by: Jes Sorensen 
---
 hw/pc.c |   13 +++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 58dea57..b17a199 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -39,6 +39,7 @@
 #include "msix.h"
 #include "sysbus.h"
 #include "sysemu.h"
+#include "kvm.h"
 
 /* output Bochs bios info messages */
 //#define DEBUG_BIOS
@@ -866,11 +867,19 @@ void pc_cpus_init(const char *cpu_model)
 
 /* init CPUs */
 if (cpu_model == NULL) {
+if (kvm_enabled()) {
 #ifdef TARGET_X86_64
-cpu_model = "qemu64";
+cpu_model = "kvm64";
 #else
-cpu_model = "qemu32";
+cpu_model = "kvm32";
 #endif
+} else {
+#ifdef TARGET_X86_64
+cpu_model = "qemu64";
+#else
+cpu_model = "qemu32";
+#endif
+}
 }
 
 for(i = 0; i < smp_cpus; i++) {
-- 
1.7.1.1




[Qemu-devel] [PATCH v2 0/2] Use kvm64/kvm32 when running under KVM

2010-07-28 Thread Jes . Sorensen
From: Jes Sorensen 

This set of patches adds default CPU types to the PC compat
definitions, and patch #2 sets the CPU type to kvm64/kvm32 when
running under KVM.

Long term we might want to qdev'ify the CPUs but I think it is better
to keep it simple for 0.13.

Jes Sorensen (2):
  Set a default CPU type for compat PC machine defs.
  Use kvm32/kvm64 as default CPUs when running under KVM.

 hw/boards.h  |1 +
 hw/pc.c  |   13 +++--
 hw/pc_piix.c |   15 +++
 vl.c |2 ++
 4 files changed, 29 insertions(+), 2 deletions(-)




[Qemu-devel] Re: [PATCH] vmstate: fix vmstate_subsection_load

2010-07-28 Thread TeLeMan
--
SUN OF A BEACH



On Wed, Jul 28, 2010 at 20:32, Juan Quintela  wrote:
> TeLeMan  wrote:
>> On Wed, Jul 28, 2010 at 19:51, Juan Quintela  wrote:
>>> I hope this hepls to understand how subsections are supposed to work.
>>>
>>> Thanks for the comments, Juan.
>>>
>> I see, thanks a lot. But I still hope to have the similar subsection
>> that can be ignored simply.
>
> Then it is better not to be sent in the 1st place.
>
> Do you have any example of why you want to do?  When I dessigned
> subsections, I looked at all the changes that we had from 0.11 to 0.12,
> subsections can handle all of them.  Just curious about what you need.\
>
> Notice that ignoring subsections at this point is not trivial, as
> (sub)sections don't have a size field.  Working on getting size there,
> but it is a long term project (it requires 1st to change everything to
> VMState to be able to change how QEMUFile works).

I have some extra data to be saved to vmstate and I want the new
vmstate to be compatible with the official version. You are right, I
thought it was simple. Now I discard my thought.



Re: [Qemu-devel] Re: KVM call agenda for July 27

2010-07-28 Thread Markus Armbruster
Kevin Wolf  writes:

> Am 28.07.2010 13:22, schrieb Markus Armbruster:
>> Anthony Liguori  writes:
>> 
>>> On 07/27/2010 10:22 AM, Markus Armbruster wrote:
[...]
 Raw can't be probed safely, by its very nature.  For historical reasons,
 we try anyway.  I think we should stop doing that, even though that
 breaks existing use relying on the misfeature.  Announce it now, spit
 out scary warnings, kill it for good 1-2 releases later.

 If we're unwilling to do that, then I'd *strongly* prefer doing nothing
 over silently messing with the raw writes to sector 0 (so does
 Christoph, and he explained why).
>>>
>>> If we add docs/deprecated-features.txt, schedule removal for at least
>>> 1 year in the future, and put a warning in the code that prints
>>> whenever raw is probed, I think I could warm up to this.
>>>
>>> Since libvirt should be insulating users from this today, I think the
>>> fall out might not be terrible.
>> 
>> Okay, I'll prepare a patch.
>
> This kills -hda and friends for raw images. I'm not sure this is a good
> idea.

Please wait and see my patch.



Re: [Qemu-devel] PATCH: Adding options to generate SCSI based VMDK images

2010-07-28 Thread Kevin Wolf
Hi Aaron,

Am 28.07.2010 04:01, schrieb Aaron Mason:
> I tried this once before using command-line parameters and it was
> knocked back.  Someone made the mention of using options so I had a
> look and saw that it was really simple to do.
> 
> The following is a patch from the latest Git snapshot.  I have also
> attached it (in case it gets broken by Gmail) and uploaded it to
> http://milo.thats-too-much.info/qemu-vmdk_gen_scsi.patch (in case the
> mailing list prohibits attachments).

Needs a Signed-off-by line. Also please use "[PATCH]" instead of
"PATCH:" in the subject line so that git am automatically removes it
when applying the patch. You get a correctly formatted mail with git
format-patch.

And yes, your mailer has broken the patch, so adding the attachment was
a good idea (using git send-email would be even better, though).

> diff --git a/block/vmdk.c b/block/vmdk.c
> index 2d4ba42..b60d86f 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -686,9 +686,9 @@ static int vmdk_create(const char *filename,
> QEMUOptionParameter *options)
>  "ddb.geometry.cylinders = \"%" PRId64 "\"\n"
>  "ddb.geometry.heads = \"16\"\n"
>  "ddb.geometry.sectors = \"63\"\n"
> -"ddb.adapterType = \"ide\"\n";
> +"ddb.adapterType = \"%s\"\n";
>  char desc[1024];
> -const char *real_filename, *temp_str;
> +const char *real_filename, *temp_str, *adapter_type = "ide";
>  int64_t total_size = 0;
>  const char *backing_file = NULL;
>  int flags = 0;
> @@ -702,6 +702,15 @@ static int vmdk_create(const char *filename,
> QEMUOptionParameter *options)
>  backing_file = options->value.s;
>  } else if (!strcmp(options->name, BLOCK_OPT_COMPAT6)) {
>  flags |= options->value.n ? BLOCK_FLAG_COMPAT6: 0;
> +} else if (!strcmp(options->name, BLOCK_OPT_ADAPTER)) {
> +if (options->value.s != NULL) {
> +if (!strcmp(options->value.s, "ide") || \
> +   !strcmp(options->value.s, "buslogic") || \
> +!strcmp(options->value.s, "lsilogic")) {
> +adapter_type = options->value.s;
> +} else
> +return -1;
> +}

qemu uses four spaces for indentation, tabs are not allowed. Also, a \
at the end of a line is not needed.

The logic of the patch looks okay, so please send a v2 with just these
formal things fixed.

Kevin



[Qemu-devel] Re: [PATCH] vmstate: fix vmstate_subsection_load

2010-07-28 Thread Paolo Bonzini

On 07/28/2010 02:51 PM, TeLeMan wrote:

On Wed, Jul 28, 2010 at 20:32, Juan Quintela  wrote:

TeLeMan  wrote:

On Wed, Jul 28, 2010 at 19:51, Juan Quintela  wrote:

I hope this hepls to understand how subsections are supposed to work.

Thanks for the comments, Juan.


I see, thanks a lot. But I still hope to have the similar subsection
that can be ignored simply.


Then it is better not to be sent in the 1st place.

Do you have any example of why you want to do?  When I dessigned
subsections, I looked at all the changes that we had from 0.11 to 0.12,
subsections can handle all of them.  Just curious about what you need.\

Notice that ignoring subsections at this point is not trivial, as
(sub)sections don't have a size field.  Working on getting size there,
but it is a long term project (it requires 1st to change everything to
VMState to be able to change how QEMUFile works).


I have some extra data to be saved to vmstate and I want the new
vmstate to be compatible with the official version. You are right, I
thought it was simple. Now I discard my thought.


Even if they are mandatory, subsections still improve the situation 
here, because they provide a clean way to "branch" off an upstream 
vmstate version.  At least the failure will be clear, because an 
unsupported subsection is easily detected when migrating to (or 
restoring with) upstream.


Instead, for example RHEL5.5's "version 9" cpu save format will often 
crash upstream version 9 with a SIGSEGV.




Re: [Qemu-devel] Re: [PATCH] Introduce a -libvirt-caps flag as a stop-gap

2010-07-28 Thread Anthony Liguori

On 07/28/2010 04:53 AM, Daniel P. Berrange wrote:

On Tue, Jul 27, 2010 at 12:20:55PM -0500, Anthony Liguori wrote:
   

On 07/27/2010 12:00 PM, Daniel P. Berrange wrote:
 

Yup.  You'll need to decide up front if you want to probe for a feature
when it's introduced and have something added to capabilities.

This is simple though.  A few weeks before 0.14 is released, go through
the change log, and anything that looks interesting, add a cap flag.

 

That doesn't work for features which already exist in QEMU which are
not yet supported in libvirt. eg consider QEMU 0.13 is released, and
then we want to add a new feature to libvirt a month later.
   

Right.  So sit down and look at the 0.13 changelog and if there's any
features that you think you might want to support at some point in time,
add a capability.
 

Not really practical - pretty much anything is a candidate because
we want to support as many of QEMU features as possible.

   

It offers significantly less information that the existing -help
data, so I don't think it is workable as a replacement. We'd get
into the bad situation where we could support a feature in 0.12
but not in 0.13, unless we went back to using -help output again.

If we're going for a short term hack, then how about a combination
of

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

   

Would have failed in exactly the same way that the current -help parsing
fails.  The description of an argument in the help text is not a
capabilities string.
 

This particular case of cache modes might have failed, but in the
broader picture it would have been more reliable. The vasty
majority of the time, we only check whether a particular named
argument exists. This patch would make it very reliable todo so
because you could simply extract a single named field from the
JSON doc. The cases where we looked at the parameter options
would have been improved a little, but still be potentially fragile.
Checking for version number would also be improved. So overall
while obviously not perfect, it would be significantly better than
the solution today and using an approach that isn't a complete
throwaway solution.
   


I'd be willing to spit out the option names minus the help 
descriptions.  The option names are part of a supported interface so 
there's no harm in exposing an interface for that.


But I think libvirt really needs option names + indication when we've 
added parameters to an option, right?


Regards,

Anthony Liguori


Regards,
Daniel
   





Re: [Qemu-devel] [PATCH v2 0/2] Use kvm64/kvm32 when running under KVM

2010-07-28 Thread Markus Armbruster
jes.soren...@redhat.com writes:

> From: Jes Sorensen 
>
> This set of patches adds default CPU types to the PC compat
> definitions, and patch #2 sets the CPU type to kvm64/kvm32 when
> running under KVM.
>
> Long term we might want to qdev'ify the CPUs but I think it is better
> to keep it simple for 0.13.

Makes sense to me.



[Qemu-devel] Re: [PATCH] vmstate: fix vmstate_subsection_load

2010-07-28 Thread Juan Quintela
Paolo Bonzini  wrote:
> On 07/28/2010 02:51 PM, TeLeMan wrote:

> Even if they are mandatory, subsections still improve the situation
> here, because they provide a clean way to "branch" off an upstream
> vmstate version.  At least the failure will be clear, because an
> unsupported subsection is easily detected when migrating to (or
> restoring with) upstream.
>
> Instead, for example RHEL5.5's "version 9" cpu save format will often
> crash upstream version 9 with a SIGSEGV.

Old kvm (pre start of merge with qemu) and qemu device versions are out
of sync.  You can't migrate between them at all.  Basically you can't
migrate from anything older than qemu-0.12.  In qemu-0.12 firmware for
devices got moved to pci bars and memory layout changed in such a way
that it is not compatible at all.

Later, Juan.



Re: [Qemu-devel] [RFC PATCH 03/14] KVM Test: Add a common ping module for network related tests

2010-07-28 Thread Michael Goldish
On 07/28/2010 02:50 PM, Michael Goldish wrote:
> On 07/27/2010 04:01 PM, Lucas Meneghel Rodrigues wrote:
>> On Tue, 2010-07-20 at 09:35 +0800, Amos Kong wrote:
>>> The kvm_net_utils.py is a just a place that wraps common network
>>> related commands which is used to do the network-related tests.
>>> Use -1 as the packet ratio for loss analysis.
>>> Use quiet mode when doing the flood ping.
>>>
>>> Signed-off-by: Jason Wang 
>>> Signed-off-by: Amos Kong 
>>> ---
>>>  0 files changed, 0 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/client/tests/kvm/kvm_net_utils.py 
>>> b/client/tests/kvm/kvm_net_utils.py
>>> index ede4965..8a71858 100644
>>> --- a/client/tests/kvm/kvm_net_utils.py
>>> +++ b/client/tests/kvm/kvm_net_utils.py
>>> @@ -1,4 +1,114 @@
>>> -import re
>>> +import logging, re, signal
>>> +from autotest_lib.client.common_lib import error
>>> +import kvm_subprocess, kvm_utils
>>> +
>>> +def get_loss_ratio(output):
>>> +"""
>>> +Get the packet loss ratio from the output of ping
>>> +
>>> +@param output
>>> +"""
>>> +try:
>>> +return int(re.findall('(\d+)% packet loss', output)[0])
>>> +except IndexError:
>>> +logging.debug(output)
>>> +return -1
>>
>>> +def raw_ping(command, timeout, session, output_func):
>>> +"""
>>> +Low-level ping command execution.
>>> +
>>> +@param command: ping command
>>> +@param timeout: timeout of the ping command
>>> +@param session: local executon hint or session to execute the ping 
>>> command
>>> +"""
>>> +if session == "localhost":
> 
> I have no problem with this, but wouldn't it be nicer to use None to
> indicate that the session should be local?
> 
>>> +process = kvm_subprocess.run_bg(command, output_func=output_func,
>>> +timeout=timeout)
>>
>> Do we really have to resort to kvm_subprocess here? We use subprocess on
>> special occasions, usually when the command in question is required to
>> live throughout the tests, which doesn't seem to be the case.
>> kvm_subprocess is a good library, but it is a more heavyweight
>> alternative (spawns its own shell process, for example). Maybe you are
>> using it because you can directly send input to the process at any time,
>> such as the ctrl+c later on this same code.
>>
>>> +
>>> +# Send SIGINT singal to notify the timeout of running ping process,
>>
>> ^ typo, signal
>>
>>> +# Because ping have the ability to catch the SIGINT signal so we 
>>> can
>>> +# always get the packet loss ratio even if timeout.
>>> +if process.is_alive():
>>> +kvm_utils.kill_process_tree(process.get_pid(), signal.SIGINT)
>>> +
>>> +status = process.get_status()
>>> +output = process.get_output()
>>> +
>>> +process.close()
>>> +return status, output
>>> +else:
>>> +session.sendline(command)
>>> +status, output = session.read_up_to_prompt(timeout=timeout,
>>> +   print_func=output_func)
>>> +if status is False:
>>
>> ^ For None objects, it is better to explicitly test for is None. However
>> the above construct seems more natural if put as
>>
>> if not status:
>>
>> Any particular reason you tested explicitely for False?
> 
> read_up_to_prompt() returns True/False as the first member of the tuple.
> 
>>> +# Send ctrl+c (SIGINT) through ssh session
>>> +session.sendline("\003")
> 
> I think you can use send() here.  sendline() calls send() with a newline
> added to the string.
> 
>>> +status, output2 = 
>>> session.read_up_to_prompt(print_func=output_func)
>>> +output += output2
>>> +if status is False:
>>> +# We also need to use this session to query the return 
>>> value
>>> +session.sendline("\003")
> 
> Same here.
> 
>>> +
>>> +session.sendline(session.status_test_command)
>>> +s2, o2 = session.read_up_to_prompt()
>>> +if s2 is False:
>>
>> ^ See comment above
>>
>>> +status = -1
>>> +else:
>>> +try:
>>> +status = int(re.findall("\d+", o2)[0])
>>> +except:
>>> +status = -1
>>> +
>>> +return status, output
>>> +
>>> +def ping(dest = "localhost", count = None, interval = None, interface = 
>>> None,
>>> + packetsize = None, ttl = None, hint = None, adaptive = False,
>>> + broadcast = False, flood = False, timeout = 0,
>>> + output_func = logging.debug, session = "localhost"):
>>
>> ^ On function headers, we pretty much follow PEP 8 and don't use spacing
>> between argument keys, the equal sign and the default value, so this
>> header should be rewritten
>>
>> def ping(dest="localhost",...)
>>
>>> +"""
>>> +Wrapper of ping.
>>> +
>>> +@param dest: destination address
>>> +@param count: count of icmp packet
>>> +@param interval: inter

Re: [Qemu-devel] [PATCH] Ignore writes of perf reg (cp15 with crm == 12)

2010-07-28 Thread Loïc Minier
 I found out Matt Waddel has written a better looking patch, but I
 didn't test it; reviews welcome -- attached

-- 
Loïc Minier
--- Begin Message ---

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 7440163..b5d8a6c 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -130,6 +130,7 @@ typedef struct CPUARMState {
 uint32_t c6_data;
 uint32_t c9_insn; /* Cache lockdown registers.  */
 uint32_t c9_data;
+uint32_t c9_pmcr_data; /* Performance Monitor Control Register */
 uint32_t c12_vbar; /* secure/nonsecure vector base address register. */
 uint32_t c12_mvbar; /* monitor vector base address register. */
 uint32_t c13_fcse; /* FCSE PID.  */
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 1f5f307..2136c07 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -1558,6 +1558,15 @@ void HELPER(set_cp15)(CPUState *env, uint32_t insn, uint32_t val)
 case 1: /* TCM memory region registers.  */
 /* Not implemented.  */
 goto bad_reg;
+case 12:
+switch (op2) {
+case 0:
+env->cp15.c9_pmcr_data = val;
+break;
+default:
+goto bad_reg;
+}
+break;
 default:
 goto bad_reg;
 }
@@ -1897,6 +1906,13 @@ uint32_t HELPER(get_cp15)(CPUState *env, uint32_t insn)
 goto bad_reg;
 /* L2 Lockdown and Auxiliary control.  */
 return 0;
+case 12:
+switch (op2) {
+case 0:
+return env->cp15.c9_pmcr_data;
+default:
+goto bad_reg;
+}
 default:
 goto bad_reg;
 }
diff --git a/target-arm/machine.c b/target-arm/machine.c
index 8595549..026776d 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -46,6 +46,7 @@ void cpu_save(QEMUFile *f, void *opaque)
 qemu_put_be32(f, env->cp15.c6_data);
 qemu_put_be32(f, env->cp15.c9_insn);
 qemu_put_be32(f, env->cp15.c9_data);
+qemu_put_be32(f, env->cp15.c9_pmcr_data);
 qemu_put_be32(f, env->cp15.c13_fcse);
 qemu_put_be32(f, env->cp15.c13_context);
 qemu_put_be32(f, env->cp15.c13_tls1);
@@ -156,6 +157,7 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
 env->cp15.c6_data = qemu_get_be32(f);
 env->cp15.c9_insn = qemu_get_be32(f);
 env->cp15.c9_data = qemu_get_be32(f);
+env->cp15.c9_pmcr_data = qemu_get_be32(f);
 env->cp15.c13_fcse = qemu_get_be32(f);
 env->cp15.c13_context = qemu_get_be32(f);
 env->cp15.c13_tls1 = qemu_get_be32(f);

--- End Message ---


Re: [Qemu-devel] qemu cp15 access

2010-07-28 Thread Loïc Minier
On Mon, Jul 26, 2010, Raymes Khoury wrote:
> I am having the problem with qemu, as described in the post
> http://old.nabble.com/-PATCH:-PR-target-42671--Use-Thumb1-GOT-address-loading-sequence-for--%09Thumb2-td27124497.html
> where
> accessing cp15 on ARM causes an error:

 See mid 1280086076-20649-1-git-send-email-loic.min...@linaro.org and
 thread

 http://article.gmane.org/gmane.comp.emulators.qemu/77092

-- 
Loïc Minier



[Qemu-devel] [Bug 595438] Re: KVM segmentation fault, using SCSI+writeback and linux 2.4 guest

2010-07-28 Thread Serge Hallyn
Thanks for pushing on this!  The fix is in 0.13.0-rc0, and we will merge
this into maverick as soon as it is GA'd.  The fix appears to very
cleanly (with a few offsets but no rejects) apply to both the lucid and
maverick source.  I propose we merge the patch by itself into lucid, and
wait for the GA for maverick.

-- 
KVM segmentation fault, using SCSI+writeback and linux 2.4 guest
https://bugs.launchpad.net/bugs/595438
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.

Status in Kernel Virtual Machine: Confirmed
Status in QEMU: Fix Committed
Status in qemu-kvm: Confirmed

Bug description:
I Use Ubuntu 32 bit 10.04 with standard KVM.
I have Intel E7600  @ 3.06GHz processor with VMX

In this system I Run:
LC_ALL=C PATH=/usr/local/sbin:/usr/local/bin:/usr/bin:/usr/sbin:/sbin:/bin 
QEMU_AUDIO_DRV=none /usr/bin/kvm -M pc-0.12 -enable-kvm -m 256 -smp 1 -name 
spamsender -uuid b9cacd5e-08f7-41fd-78c8-89cec59af881 -chardev 
socket,id=monitor,path=/var/lib/libvirt/qemu/spamsender.monitor,server,nowait 
-monitor chardev:monitor -boot d -drive 
file=/mnt/megadiff/cdiso_400_130.iso,if=ide,media=cdrom,index=2 -drive 
file=/home/mmarkk/spamsender2.img,if=scsi,index=0,format=qcow2,cache=writeback 
-net nic,macaddr=00:00:00:00:00:00,vlan=0,name=nic.0 -net tap,vlan=0,name=tap.0 
-chardev pty,id=serial0 -serial chardev:serial0 -parallel none -usb -vnc 
127.0.0.1:0 -vga cirrus

.iso image contain custom distro of 2.4-linux kernel based system. During 
install process (when .tar.gz actively unpacked), kvm dead with segmentation 
fault.

And ONLY when I choose scsi virtual disk and writeback simultaneously. 
But, writeback+ide, writethrough+scsi works OK.

I use qcow2. It seems, that qcow does not have such problems.

Virtual machine get down at random time during file copy. It seems, when qcow2 
file size need to be expanded.







[Qemu-devel] [PATCH 0/3] vhost: mergeable buffer support

2010-07-28 Thread Michael S. Tsirkin
This adds support for mergeable buffers with vhost.
The tun ioctl this relies on is already in 2.6.34,
the vhost support for the feature has been submitted
for 2.6.36.

Michael S. Tsirkin (3):
  tap: generalize code for different vnet header len
  tap: add APIs for vnet header length
  vhost_net: mergeable buffers support

 hw/vhost_net.c|   24 +++-
 net/tap-aix.c |9 +
 net/tap-bsd.c |9 +
 net/tap-linux.c   |   29 +
 net/tap-linux.h   |8 
 net/tap-solaris.c |9 +
 net/tap-win32.c   |9 +
 net/tap.c |   49 +++--
 net/tap.h |4 
 9 files changed, 135 insertions(+), 15 deletions(-)

-- 
MST



[Qemu-devel] [PATCH 3/3] vhost_net: mergeable buffers support

2010-07-28 Thread Michael S. Tsirkin
use the new tap APIs to set header length

Signed-off-by: Michael S. Tsirkin 
---
 hw/vhost_net.c |   24 +++-
 1 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/hw/vhost_net.c b/hw/vhost_net.c
index 606aa0c..3bd3aa6 100644
--- a/hw/vhost_net.c
+++ b/hw/vhost_net.c
@@ -51,7 +51,9 @@ unsigned vhost_net_get_features(struct vhost_net *net, 
unsigned features)
 if (!(net->dev.features & (1 << VIRTIO_RING_F_INDIRECT_DESC))) {
 features &= ~(1 << VIRTIO_RING_F_INDIRECT_DESC);
 }
-features &= ~(1 << VIRTIO_NET_F_MRG_RXBUF);
+if (!(net->dev.features & (1 << VIRTIO_NET_F_MRG_RXBUF))) {
+features &= ~(1 << VIRTIO_NET_F_MRG_RXBUF);
+}
 return features;
 }
 
@@ -64,6 +66,9 @@ void vhost_net_ack_features(struct vhost_net *net, unsigned 
features)
 if (features & (1 << VIRTIO_RING_F_INDIRECT_DESC)) {
 net->dev.acked_features |= (1 << VIRTIO_RING_F_INDIRECT_DESC);
 }
+if (features & (1 << VIRTIO_NET_F_MRG_RXBUF)) {
+net->dev.acked_features |= (1 << VIRTIO_NET_F_MRG_RXBUF);
+}
 }
 
 static int vhost_net_get_fd(VLANClientState *backend)
@@ -98,6 +103,10 @@ struct vhost_net *vhost_net_init(VLANClientState *backend, 
int devfd)
 if (r < 0) {
 goto fail;
 }
+if (!tap_has_vnet_hdr_len(backend,
+  sizeof(struct virtio_net_hdr_mrg_rxbuf))) {
+net->dev.features &= ~(1 << VIRTIO_NET_F_MRG_RXBUF);
+}
 if (~net->dev.features & net->dev.backend_features) {
 fprintf(stderr, "vhost lacks feature mask %" PRIu64 " for backend\n",
 (uint64_t)(~net->dev.features & net->dev.backend_features));
@@ -118,6 +127,10 @@ int vhost_net_start(struct vhost_net *net,
 {
 struct vhost_vring_file file = { };
 int r;
+if (net->dev.acked_features & (1 << VIRTIO_NET_F_MRG_RXBUF)) {
+tap_set_vnet_hdr_len(net->vc,
+ sizeof(struct virtio_net_hdr_mrg_rxbuf));
+}
 
 net->dev.nvqs = 2;
 net->dev.vqs = net->vqs;
@@ -145,6 +158,9 @@ fail:
 }
 net->vc->info->poll(net->vc, true);
 vhost_dev_stop(&net->dev, dev);
+if (net->dev.acked_features & (1 << VIRTIO_NET_F_MRG_RXBUF)) {
+tap_set_vnet_hdr_len(net->vc, sizeof(struct virtio_net_hdr));
+}
 return r;
 }
 
@@ -159,11 +175,17 @@ void vhost_net_stop(struct vhost_net *net,
 }
 net->vc->info->poll(net->vc, true);
 vhost_dev_stop(&net->dev, dev);
+if (net->dev.acked_features & (1 << VIRTIO_NET_F_MRG_RXBUF)) {
+tap_set_vnet_hdr_len(net->vc, sizeof(struct virtio_net_hdr));
+}
 }
 
 void vhost_net_cleanup(struct vhost_net *net)
 {
 vhost_dev_cleanup(&net->dev);
+if (net->dev.acked_features & (1 << VIRTIO_NET_F_MRG_RXBUF)) {
+tap_set_vnet_hdr_len(net->vc, sizeof(struct virtio_net_hdr));
+}
 qemu_free(net);
 }
 #else
-- 
1.7.2.rc0.14.g41c1c



[Qemu-devel] [PATCH 1/3] tap: generalize code for different vnet header len

2010-07-28 Thread Michael S. Tsirkin
Make host vnet header length a structure field in
preparation for using this support in linux kernel.

Signed-off-by: Michael S. Tsirkin 
---
 net/tap-linux.h |6 ++
 net/tap.c   |   28 ++--
 2 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/net/tap-linux.h b/net/tap-linux.h
index 9f94358..727fcf5 100644
--- a/net/tap-linux.h
+++ b/net/tap-linux.h
@@ -52,4 +52,10 @@ struct virtio_net_hdr
 uint16_t csum_offset;
 };
 
+struct virtio_net_hdr_mrg_rxbuf
+{
+struct virtio_net_hdr hdr;
+uint16_t num_buffers;   /* Number of merged rx buffers */
+};
+
 #endif /* QEMU_TAP_H */
diff --git a/net/tap.c b/net/tap.c
index 0147dab..2866ff4 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -57,10 +57,10 @@ typedef struct TAPState {
 uint8_t buf[TAP_BUFSIZE];
 unsigned int read_poll : 1;
 unsigned int write_poll : 1;
-unsigned int has_vnet_hdr : 1;
 unsigned int using_vnet_hdr : 1;
 unsigned int has_ufo: 1;
 VHostNetState *vhost_net;
+unsigned host_vnet_hdr_len;
 } TAPState;
 
 static int launch_script(const char *setup_script, const char *ifname, int fd);
@@ -121,11 +121,11 @@ static ssize_t tap_receive_iov(VLANClientState *nc, const 
struct iovec *iov,
 TAPState *s = DO_UPCAST(TAPState, nc, nc);
 const struct iovec *iovp = iov;
 struct iovec iov_copy[iovcnt + 1];
-struct virtio_net_hdr hdr = { 0, };
+struct virtio_net_hdr_mrg_rxbuf hdr = { };
 
-if (s->has_vnet_hdr && !s->using_vnet_hdr) {
+if (s->host_vnet_hdr_len && !s->using_vnet_hdr) {
 iov_copy[0].iov_base = &hdr;
-iov_copy[0].iov_len =  sizeof(hdr);
+iov_copy[0].iov_len =  s->host_vnet_hdr_len;
 memcpy(&iov_copy[1], iov, iovcnt * sizeof(*iov));
 iovp = iov_copy;
 iovcnt++;
@@ -139,11 +139,11 @@ static ssize_t tap_receive_raw(VLANClientState *nc, const 
uint8_t *buf, size_t s
 TAPState *s = DO_UPCAST(TAPState, nc, nc);
 struct iovec iov[2];
 int iovcnt = 0;
-struct virtio_net_hdr hdr = { 0, };
+struct virtio_net_hdr_mrg_rxbuf hdr = { };
 
-if (s->has_vnet_hdr) {
+if (s->host_vnet_hdr_len) {
 iov[iovcnt].iov_base = &hdr;
-iov[iovcnt].iov_len  = sizeof(hdr);
+iov[iovcnt].iov_len  = s->host_vnet_hdr_len;
 iovcnt++;
 }
 
@@ -159,7 +159,7 @@ static ssize_t tap_receive(VLANClientState *nc, const 
uint8_t *buf, size_t size)
 TAPState *s = DO_UPCAST(TAPState, nc, nc);
 struct iovec iov[1];
 
-if (s->has_vnet_hdr && !s->using_vnet_hdr) {
+if (s->host_vnet_hdr_len && !s->using_vnet_hdr) {
 return tap_receive_raw(nc, buf, size);
 }
 
@@ -202,9 +202,9 @@ static void tap_send(void *opaque)
 break;
 }
 
-if (s->has_vnet_hdr && !s->using_vnet_hdr) {
-buf  += sizeof(struct virtio_net_hdr);
-size -= sizeof(struct virtio_net_hdr);
+if (s->host_vnet_hdr_len && !s->using_vnet_hdr) {
+buf  += s->host_vnet_hdr_len;
+size -= s->host_vnet_hdr_len;
 }
 
 size = qemu_send_packet_async(&s->nc, buf, size, tap_send_completed);
@@ -229,7 +229,7 @@ int tap_has_vnet_hdr(VLANClientState *nc)
 
 assert(nc->info->type == NET_CLIENT_TYPE_TAP);
 
-return s->has_vnet_hdr;
+return !!s->host_vnet_hdr_len;
 }
 
 void tap_using_vnet_hdr(VLANClientState *nc, int using_vnet_hdr)
@@ -239,7 +239,7 @@ void tap_using_vnet_hdr(VLANClientState *nc, int 
using_vnet_hdr)
 using_vnet_hdr = using_vnet_hdr != 0;
 
 assert(nc->info->type == NET_CLIENT_TYPE_TAP);
-assert(s->has_vnet_hdr == using_vnet_hdr);
+assert(!!s->host_vnet_hdr_len == using_vnet_hdr);
 
 s->using_vnet_hdr = using_vnet_hdr;
 }
@@ -310,7 +310,7 @@ static TAPState *net_tap_fd_init(VLANState *vlan,
 s = DO_UPCAST(TAPState, nc, nc);
 
 s->fd = fd;
-s->has_vnet_hdr = vnet_hdr != 0;
+s->host_vnet_hdr_len = vnet_hdr ? sizeof(struct virtio_net_hdr) : 0;
 s->using_vnet_hdr = 0;
 s->has_ufo = tap_probe_has_ufo(s->fd);
 tap_set_offload(&s->nc, 0, 0, 0, 0, 0);
-- 
1.7.2.rc0.14.g41c1c




[Qemu-devel] [PATCH 2/3] tap: add APIs for vnet header length

2010-07-28 Thread Michael S. Tsirkin
Add APIs to control host header length. First user
will be vhost-net.

Signed-off-by: Michael S. Tsirkin 
---
 net/tap-aix.c |9 +
 net/tap-bsd.c |9 +
 net/tap-linux.c   |   29 +
 net/tap-linux.h   |2 ++
 net/tap-solaris.c |9 +
 net/tap-win32.c   |9 +
 net/tap.c |   21 +
 net/tap.h |4 
 8 files changed, 92 insertions(+), 0 deletions(-)

diff --git a/net/tap-aix.c b/net/tap-aix.c
index 4bc9f2f..e19aaba 100644
--- a/net/tap-aix.c
+++ b/net/tap-aix.c
@@ -46,6 +46,15 @@ int tap_probe_has_ufo(int fd)
 return 0;
 }
 
+int tap_probe_vnet_hdr_len(int fd, int len)
+{
+return 0;
+}
+
+void tap_fd_set_vnet_hdr_len(int fd, int len)
+{
+}
+
 void tap_fd_set_offload(int fd, int csum, int tso4,
 int tso6, int ecn, int ufo)
 {
diff --git a/net/tap-bsd.c b/net/tap-bsd.c
index 3773d5d..3513075 100644
--- a/net/tap-bsd.c
+++ b/net/tap-bsd.c
@@ -116,6 +116,15 @@ int tap_probe_has_ufo(int fd)
 return 0;
 }
 
+int tap_probe_vnet_hdr_len(int fd, int len)
+{
+return 0;
+}
+
+void tap_fd_set_vnet_hdr_len(int fd, int len)
+{
+}
+
 void tap_fd_set_offload(int fd, int csum, int tso4,
 int tso6, int ecn, int ufo)
 {
diff --git a/net/tap-linux.c b/net/tap-linux.c
index 03b8301..a425ff0 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -127,6 +127,35 @@ int tap_probe_has_ufo(int fd)
 return 1;
 }
 
+/* Verify that we can assign given length */
+int tap_probe_vnet_hdr_len(int fd, int len)
+{
+int orig;
+if (ioctl(fd, TUNGETVNETHDRSZ, &orig) == -1) {
+return 0;
+}
+if (ioctl(fd, TUNSETVNETHDRSZ, &len) == -1) {
+return 0;
+}
+/* Restore original length: we can't handle failure. */
+if (ioctl(fd, TUNSETVNETHDRSZ, &orig) == -1) {
+fprintf(stderr, "TUNGETVNETHDRSZ ioctl() failed: %s. Exiting.\n",
+strerror(errno));
+assert(0);
+return -errno;
+}
+return 1;
+}
+
+void tap_fd_set_vnet_hdr_len(int fd, int len)
+{
+if (ioctl(fd, TUNSETVNETHDRSZ, &len) == -1) {
+fprintf(stderr, "TUNSETVNETHDRSZ ioctl() failed: %s. Exiting.\n",
+strerror(errno));
+assert(0);
+}
+}
+
 void tap_fd_set_offload(int fd, int csum, int tso4,
 int tso6, int ecn, int ufo)
 {
diff --git a/net/tap-linux.h b/net/tap-linux.h
index 727fcf5..659e981 100644
--- a/net/tap-linux.h
+++ b/net/tap-linux.h
@@ -27,6 +27,8 @@
 #define TUNSETOFFLOAD  _IOW('T', 208, unsigned int)
 #define TUNGETIFF  _IOR('T', 210, unsigned int)
 #define TUNSETSNDBUF   _IOW('T', 212, int)
+#define TUNGETVNETHDRSZ _IOR('T', 215, int)
+#define TUNSETVNETHDRSZ _IOW('T', 216, int)
 
 #endif
 
diff --git a/net/tap-solaris.c b/net/tap-solaris.c
index 50d127a..ca1d496 100644
--- a/net/tap-solaris.c
+++ b/net/tap-solaris.c
@@ -211,6 +211,15 @@ int tap_probe_has_ufo(int fd)
 return 0;
 }
 
+int tap_probe_vnet_hdr_len(int fd, int len)
+{
+return 0;
+}
+
+void tap_fd_set_vnet_hdr_len(int fd, int len)
+{
+}
+
 void tap_fd_set_offload(int fd, int csum, int tso4,
 int tso6, int ecn, int ufo)
 {
diff --git a/net/tap-win32.c b/net/tap-win32.c
index 74348da..9fe4fcd 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -728,6 +728,15 @@ int tap_has_vnet_hdr(VLANClientState *vc)
 return 0;
 }
 
+int tap_probe_vnet_hdr_len(int fd, int len)
+{
+return 0;
+}
+
+void tap_fd_set_vnet_hdr_len(int fd, int len)
+{
+}
+
 void tap_using_vnet_hdr(VLANClientState *vc, int using_vnet_hdr)
 {
 }
diff --git a/net/tap.c b/net/tap.c
index 2866ff4..4afb314 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -232,6 +232,27 @@ int tap_has_vnet_hdr(VLANClientState *nc)
 return !!s->host_vnet_hdr_len;
 }
 
+int tap_has_vnet_hdr_len(VLANClientState *nc, int len)
+{
+TAPState *s = DO_UPCAST(TAPState, nc, nc);
+
+assert(nc->info->type == NET_CLIENT_TYPE_TAP);
+
+return tap_probe_vnet_hdr_len(s->fd, len);
+}
+
+void tap_set_vnet_hdr_len(VLANClientState *nc, int len)
+{
+TAPState *s = DO_UPCAST(TAPState, nc, nc);
+
+assert(nc->info->type == NET_CLIENT_TYPE_TAP);
+assert(len == sizeof(struct virtio_net_hdr_mrg_rxbuf) ||
+   len == sizeof(struct virtio_net_hdr));
+
+tap_fd_set_vnet_hdr_len(s->fd, len);
+s->host_vnet_hdr_len = len;
+}
+
 void tap_using_vnet_hdr(VLANClientState *nc, int using_vnet_hdr)
 {
 TAPState *s = DO_UPCAST(TAPState, nc, nc);
diff --git a/net/tap.h b/net/tap.h
index b8cec83..e44bd2b 100644
--- a/net/tap.h
+++ b/net/tap.h
@@ -40,13 +40,17 @@ ssize_t tap_read_packet(int tapfd, uint8_t *buf, int 
maxlen);
 
 int tap_has_ufo(VLANClientState *vc);
 int tap_has_vnet_hdr(VLANClientState *vc);
+int tap_has_vnet_hdr_len(VLANClientState *vc, int len);
 void tap_using_vnet_hdr(VLANClientState *vc, int using_vnet_hdr);
 void tap_set_offload(VLANClientState *vc, int csum, int tso4, int tso6, int 
ecn, int ufo);
+v

[Qemu-devel] [PATCH 2/2] Remove guest triggerable abort()

2010-07-28 Thread Gleb Natapov
This abort() condition is easily triggerable by a guest if it configures
pci bar with unaligned address that overlaps main memory.

Signed-off-by: Gleb Natapov 
---
 kvm-all.c |   16 
 1 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index fec6d05..ad46b10 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -437,18 +437,10 @@ static void kvm_set_phys_mem(target_phys_addr_t 
start_addr,
 KVMSlot *mem, old;
 int err;
 
-if (start_addr & ~TARGET_PAGE_MASK) {
-if (flags >= IO_MEM_UNASSIGNED) {
-if (!kvm_lookup_overlapping_slot(s, start_addr,
- start_addr + size)) {
-return;
-}
-fprintf(stderr, "Unaligned split of a KVM memory slot\n");
-} else {
-fprintf(stderr, "Only page-aligned memory slots supported\n");
-}
-abort();
-}
+/* kvm works in page size chunks, but the function may be called
+   with sub-page size and analigned start address. */
+size = TARGET_PAGE_ALIGN(size);
+start_addr = TARGET_PAGE_ALIGN(start_addr);
 
 /* KVM does not support read-only slots */
 phys_offset &= ~IO_MEM_ROM;
-- 
1.7.1




[Qemu-devel] [PATCH 0/2] cpu_register_physical_memory() is completely broken.

2010-07-28 Thread Gleb Natapov
Or just a little bit?

Nothing prevents guest from configuring pci mmio bar to overlap system
memory region and the physical memory address will became mmio, but
when guest will change pci bar mapping the physical address location
will not become memory again, but instead it becomes unassigned. Yes,
guest can only hurt itself by doing this, but real HW works different,
so things that may work on real HW will break in qemu.

Anyway attached are two patches that fix more pressing issues: segfault and
abourt() that can be triggered by a guest.

To trigger segfaul run Linux in qemu tcg (or apply patch 2 and then kvm
can be used too) with standard config. In the guest do the following:
# setpci -s 00:03.0 0x14.L=0xc000
# dd if=/dev/zero of=/dev/mem bs=4096 count=1 seek=12


To trigger abort run Linux in qemu with kvm and do:
# setpci -s 00:03.0 0x14.L=0xc000

Gleb Natapov (2):
  Fix segfault in mmio subpage handling code.
  Remove guest triggerable abort()

 exec.c|2 ++
 kvm-all.c |   16 
 2 files changed, 6 insertions(+), 12 deletions(-)




[Qemu-devel] [PATCH 1/2] Fix segfault in mmio subpage handling code.

2010-07-28 Thread Gleb Natapov
It is possible that subpage mmio is registered over existing memory
page. When this happens "memory" will have real memory address and not
index into io_mem array so next access to the page will generate
segfault. It is uncommon to have some part of a page to be accessed as
memory and some as mmio, but qemu shouldn't crash even when guest does
stupid things. So lets just pretend that the rest of the page is
unassigned if guest configure part of the memory page as mmio.

Signed-off-by: Gleb Natapov 
---
 exec.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/exec.c b/exec.c
index 5e9a5b7..5945496 100644
--- a/exec.c
+++ b/exec.c
@@ -3363,6 +3363,8 @@ static int subpage_register (subpage_t *mmio, uint32_t 
start, uint32_t end,
mmio, start, end, idx, eidx, memory);
 #endif
 memory = (memory >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
+if ((memory & ~TARGET_PAGE_MASK) == IO_MEM_RAM)
+memory = IO_MEM_UNASSIGNED;
 for (; idx <= eidx; idx++) {
 mmio->sub_io_index[idx] = memory;
 mmio->region_offset[idx] = region_offset;
-- 
1.7.1




Re: [Qemu-devel] [PATCH] monitor: make 'info snapshots' show only fully available snapshots

2010-07-28 Thread Markus Armbruster
Miguel Di Ciurcio Filho  writes:

> The output generated by 'info snapshots' shows only snapshots that exist on 
> the
> block device that saves the VM state. This output can cause an user to
> erroneously try to load an snapshot that is not available on all block 
> devices.

What happens when you try that?

> $ qemu-img snapshot -l xxtest.qcow2
> Snapshot list:
> IDTAG VM SIZEDATE   VM CLOCK
> 11.5M 2010-07-26 16:51:52   00:00:08.599
> 21.5M 2010-07-26 16:51:53   00:00:09.719
> 31.5M 2010-07-26 17:26:49   00:00:13.245
> 41.5M 2010-07-26 19:01:00   00:00:46.763
>
> $ qemu-img snapshot -l xxtest2.qcow2
> Snapshot list:
> IDTAG VM SIZEDATE   VM CLOCK
> 3   0 2010-07-26 17:26:49   00:00:13.245
> 4   0 2010-07-26 19:01:00   00:00:46.763
>
> Current output:
> $ qemu -hda xxtest.qcow2 -hdb xxtest2.qcow2 -monitor stdio -vnc :0
> QEMU 0.12.4 monitor - type 'help' for more information
> (qemu) info snapshots
> Snapshot devices: ide0-hd0
> Snapshot list (from ide0-hd0):
> IDTAG VM SIZEDATE   VM CLOCK
> 11.5M 2010-07-26 16:51:52   00:00:08.599
> 21.5M 2010-07-26 16:51:53   00:00:09.719
> 31.5M 2010-07-26 17:26:49   00:00:13.245
> 41.5M 2010-07-26 19:01:00   00:00:46.763
>
> Snapshots 1 and 2 do not exist on xxtest2.qcow, but they are displayed anyway.
>
> This patch sumarizes the output to only show fully available snapshots.
>
> New output:
> (qemu) info snapshots
> IDTAG VM SIZEDATE   VM CLOCK
> 31.5M 2010-07-26 17:26:49   00:00:13.245
> 41.5M 2010-07-26 19:01:00   00:00:46.763
>
> Signed-off-by: Miguel Di Ciurcio Filho 

No information on "partial" snapshots.  I doubt anybody will miss it.

> ---
>  savevm.c |   65 ++---
>  1 files changed, 45 insertions(+), 20 deletions(-)
>
> diff --git a/savevm.c b/savevm.c
> index 7a1de3c..be83878 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -1997,37 +1997,62 @@ void do_delvm(Monitor *mon, const QDict *qdict)
>  
>  void do_info_snapshots(Monitor *mon)
>  {
> -BlockDriverState *bs, *bs1;
> -QEMUSnapshotInfo *sn_tab, *sn;
> -int nb_sns, i;
> +BlockDriverState *bs_vm_state, *bs;
> +QEMUSnapshotInfo *sn_tab, *sn, s, *sn_info = &s;
> +int nb_sns, i, ret, available;
> +int total;
> +int *available_snapshots;
>  char buf[256];
>  
> -bs = bdrv_snapshots();
> -if (!bs) {
> +bs_vm_state = bdrv_snapshots();
> +if (!bs_vm_state) {
>  monitor_printf(mon, "No available block device supports 
> snapshots\n");
>  return;
>  }
> -monitor_printf(mon, "Snapshot devices:");
> -bs1 = NULL;
> -while ((bs1 = bdrv_next(bs1))) {
> -if (bdrv_can_snapshot(bs1)) {
> -if (bs == bs1)
> -monitor_printf(mon, " %s", bdrv_get_device_name(bs1));
> -}
> -}
> -monitor_printf(mon, "\n");
>  
> -nb_sns = bdrv_snapshot_list(bs, &sn_tab);
> +nb_sns = bdrv_snapshot_list(bs_vm_state, &sn_tab);
>  if (nb_sns < 0) {
>  monitor_printf(mon, "bdrv_snapshot_list: error %d\n", nb_sns);
>  return;
> +} else if (nb_sns == 0) {
> +monitor_printf(mon, "There is no snapshot available.\n");
>  }

This changes output for the "no snapshots available" case from the empty
table

IDTAG VM SIZEDATE   VM CLOCK

to

There is no snapshot available.

I'd prefer that as separate patch, if at all.

Nitpick: I don't like "return; else".

> -monitor_printf(mon, "Snapshot list (from %s):\n",
> -   bdrv_get_device_name(bs));
> -monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
> -for(i = 0; i < nb_sns; i++) {
> +
> +available_snapshots = qemu_mallocz(sizeof(int) * nb_sns);

This can die due to the nonsensical semantics of qemu_mallocz(0).

> +total = 0;
> +for (i = 0; i < nb_sns; i++) {
>  sn = &sn_tab[i];
> -monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), 
> sn));
> +available = 1;
> +bs = NULL;
> +
> +while ((bs = bdrv_next(bs))) {
> +if (bdrv_can_snapshot(bs) && bs != bs_vm_state) {
> +ret = bdrv_snapshot_find(bs, sn_info, sn->id_str);
> +if (ret < 0) {
> +available = 0;
> +break;
> +}
> +}
> +}
> +
> +if (available) {
> +available_snapshots[total] = i;

Re: [Qemu-devel] [PATCH] monitor: make 'info snapshots' show only fully available snapshots

2010-07-28 Thread Miguel Di Ciurcio Filho
On Wed, Jul 28, 2010 at 12:38 PM, Markus Armbruster  wrote:
> Miguel Di Ciurcio Filho  writes:
>
>> The output generated by 'info snapshots' shows only snapshots that exist on 
>> the
>> block device that saves the VM state. This output can cause an user to
>> erroneously try to load an snapshot that is not available on all block 
>> devices.
>
> What happens when you try that?
>

I've sent a patch that will protect that from happening [1]. With that
patch, the VM stays stopped, without it the VM keeps running with a
failed bdrv_snapshot_goto().

>
>> ---
>>  savevm.c |   65 
>> ++---
>>  1 files changed, 45 insertions(+), 20 deletions(-)
>>
>> diff --git a/savevm.c b/savevm.c
>> index 7a1de3c..be83878 100644
>> --- a/savevm.c
>> +++ b/savevm.c
>> @@ -1997,37 +1997,62 @@ void do_delvm(Monitor *mon, const QDict *qdict)
>>
>>  void do_info_snapshots(Monitor *mon)
>>  {
>> -    BlockDriverState *bs, *bs1;
>> -    QEMUSnapshotInfo *sn_tab, *sn;
>> -    int nb_sns, i;
>> +    BlockDriverState *bs_vm_state, *bs;
>> +    QEMUSnapshotInfo *sn_tab, *sn, s, *sn_info = &s;
>> +    int nb_sns, i, ret, available;
>> +    int total;
>> +    int *available_snapshots;
>>      char buf[256];
>>
>> -    bs = bdrv_snapshots();
>> -    if (!bs) {
>> +    bs_vm_state = bdrv_snapshots();
>> +    if (!bs_vm_state) {
>>          monitor_printf(mon, "No available block device supports 
>> snapshots\n");
>>          return;
>>      }
>> -    monitor_printf(mon, "Snapshot devices:");
>> -    bs1 = NULL;
>> -    while ((bs1 = bdrv_next(bs1))) {
>> -        if (bdrv_can_snapshot(bs1)) {
>> -            if (bs == bs1)
>> -                monitor_printf(mon, " %s", bdrv_get_device_name(bs1));
>> -        }
>> -    }
>> -    monitor_printf(mon, "\n");
>>
>> -    nb_sns = bdrv_snapshot_list(bs, &sn_tab);
>> +    nb_sns = bdrv_snapshot_list(bs_vm_state, &sn_tab);
>>      if (nb_sns < 0) {
>>          monitor_printf(mon, "bdrv_snapshot_list: error %d\n", nb_sns);
>>          return;
>> +    } else if (nb_sns == 0) {
>> +        monitor_printf(mon, "There is no snapshot available.\n");
>>      }
>
> This changes output for the "no snapshots available" case from the empty
> table
>
>    ID        TAG                 VM SIZE                DATE       VM CLOCK
>
> to
>
>    There is no snapshot available.
>
> I'd prefer that as separate patch, if at all.

I think a clear message saying "there is nothing" is better than an
empty table. I'm already changing the output to something more
reasonable, so.

>
> Nitpick: I don't like "return; else".
>

Yeah, kinda ugly. I will fix it.

>> -    monitor_printf(mon, "Snapshot list (from %s):\n",
>> -                   bdrv_get_device_name(bs));
>> -    monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
>> -    for(i = 0; i < nb_sns; i++) {
>> +
>> +    available_snapshots = qemu_mallocz(sizeof(int) * nb_sns);
>
> This can die due to the nonsensical semantics of qemu_mallocz(0).
>

Will fix that, so this code will be reached only if  nb_sns > 0 and
qemu_mallocz(0) will never be executed.

>> +    total = 0;
>> +    for (i = 0; i < nb_sns; i++) {
>>          sn = &sn_tab[i];
>> -        monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), 
>> sn));
>> +        available = 1;
>> +        bs = NULL;
>> +
>> +        while ((bs = bdrv_next(bs))) {
>> +            if (bdrv_can_snapshot(bs) && bs != bs_vm_state) {
>> +                ret = bdrv_snapshot_find(bs, sn_info, sn->id_str);
>> +                if (ret < 0) {
>> +                    available = 0;
>> +                    break;
>> +                }
>> +            }
>> +        }
>> +
>> +        if (available) {
>> +            available_snapshots[total] = i;
>> +            total++;
>> +        }
>>      }
>> +
>> +    if (total > 0) {
>> +        monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), 
>> NULL));
>> +        for (i = 0; i < total; i++) {
>> +            sn = &sn_tab[available_snapshots[i]];
>> +            monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, 
>> sizeof(buf), sn));
>> +        }
>> +
>> +        qemu_free(available_snapshots);
>> +
>> +    } else {
>> +        monitor_printf(mon, "There is no suitable snapshot available to be 
>> loaded.\n");
>
> Where is available_snapshots freed when control flows through this point?
>

Oops. I will fix that too.

Thanks for the feedback.

Regards,

Miguel

[1] http://lists.gnu.org/archive/html/qemu-devel/2010-07/msg01065.html
(Kevin has applied it to this block branch)



Re: [Qemu-devel] [RFC PATCH 07/14] KVM-test: Add a subtest of load/unload nic driver

2010-07-28 Thread Lucas Meneghel Rodrigues
On Tue, 2010-07-20 at 09:35 +0800, Amos Kong wrote:
> Repeatedly load/unload nic driver, try to transfer file between guest and host
> by threads at the same time, and check the md5sum.
> 
> Signed-off-by: Amos Kong 
> ---
>  0 files changed, 0 insertions(+), 0 deletions(-)
> 
> diff --git a/client/tests/kvm/tests/nicdriver_unload.py 
> b/client/tests/kvm/tests/nicdriver_unload.py
> new file mode 100644
> index 000..22f9f44
> --- /dev/null
> +++ b/client/tests/kvm/tests/nicdriver_unload.py
> @@ -0,0 +1,128 @@
> +import logging, commands, threading, re, os
> +from autotest_lib.client.common_lib import error
> +import kvm_utils, kvm_test_utils, kvm_net_utils
> +
> +def run_nicdriver_unload(test, params, env):
> +"""
> +Test nic driver
> +
> +1) Boot a vm
> +2) Get the nic driver name
> +3) Repeatedly unload/load nic driver
> +4) Multi-session TCP transfer on test interface
> +5) Check the test interface should still work
> +
> +@param test: KVM test object
> +@param params: Dictionary with the test parameters
> +@param env: Dictionary with test environment.
> +"""
> +timeout = int(params.get("login_timeout", 360))
> +vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
> +session = kvm_test_utils.wait_for_login(vm, timeout=timeout)
> +logging.info("Trying to log into guest '%s' by serial", vm.name)
> +session2 = kvm_utils.wait_for(lambda: vm.serial_login(),
> +  timeout, 0, step=2)
> +if not session2:
> +raise error.TestFail("Could not log into guest '%s'" % vm.name)
> +
> +ethname = kvm_net_utils.get_linux_ifname(session, vm.get_macaddr(0))
> +try:
> +# FIXME: Try three waies to get nic driver name, because the
> +# modprobe.conf is dropped in latest system, and ethtool method is 
> not
> +# supported by virtio_nic.
> +
> +output = session.get_command_output("cat /etc/modprobe.conf")
> +driver = re.findall(r'%s (\w+)' % ethname,output)
> +if not driver:
> +output = session.get_command_output("ethtool -i %s" % ethname)
> +driver = re.findall(r'driver: (\w+)', output)
> +if not driver:
> +output = session.get_command_output("lspci -k")
> +driver = re.findall("Ethernet controller.*\n.*\n.*Kernel driver"
> +" in use: (\w+)", output)
> +driver = driver[0]
> +except IndexError:
> +raise error.TestError("Could not find driver name")

^ About this whole block where there's an attempt to discover what the
driver name is. The methods listed there are not reliable (depends on
ethtool being installed, lspci -k not allways will list the kernel
driver in use, /etc/modprobe.conf will not be present on more recent
distributions). Instead of these methods, why don't we have a variant
for this test on each linux distro definition block, with the
appropriate driver names? It'd be something along the lines:

- 13.64:
image_name = f13-64
cdrom_cd1 = linux/Fedora-13-x86_64-DVD.iso
md5sum = 6fbae6379cf27f36e1f2c7827ba7dc35
md5sum_1m = 68821b9de4d3b5975d6634334e7f47a6
unattended_install:
unattended_file = unattended/Fedora-13.ks
tftp = images/f13-64/tftpboot
floppy = images/f13-64/floppy.img
nicdriver_unload:
e1000:
driver = e1000c
virtio:
driver = virtio_net
rtl8139:
driver = rtl8939

I believe it's safer than having to rely on the above methods.

> +
> +logging.info("driver is %s" % driver)
> +
> +class ThreadScp(threading.Thread):
> +def run(self):
> +remote_file = '/tmp/' + self.getName()
> +file_list.append(remote_file)
> +ret = vm.copy_files_to(file_name, remote_file, 
> timeout=scp_timeout)
> +logging.debug("Copy result of %s: %s" % (remote_file, ret))

^ Here it'd be worth to have 2 debug messages,

if ret:
logging.debug("File %s was transfered successfuly", remote_file)
else:
logging.debug("Failed to transfer file %s", remote_file)

> +def compare(origin_file, receive_file):
> +cmd = "md5sum %s"
> +output1 = commands.getstatusoutput(cmd % origin_file)[1].strip()

^ If we are only interested on the output, getoutput() could be used.
But in this case we care whether md5 succeeded or not, so better to do
appropriate return code treatment, as you do below. Also, you could use
utils.hash() instead of using directly md5, and that *must* yield that
exact same result, being faster than resorting to subprocesses.

Also, remembering that I do

[Qemu-devel] [PATCH 1/3] cleanup: bdrv_snaphost_find() returns zero or -ENOENT

2010-07-28 Thread Miguel Di Ciurcio Filho
The bdrv_snaphost_find() returns zero in case it finds an snapshot or -ENOENT in
case it doesn't.

Checking returning values as >= zero doesn't make sense.

Signed-off-by: Miguel Di Ciurcio Filho 
---
 savevm.c |7 ---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/savevm.c b/savevm.c
index 7a1de3c..6c6adb0 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1768,7 +1768,7 @@ static int del_existing_snapshots(Monitor *mon, const 
char *name)
 bs = NULL;
 while ((bs = bdrv_next(bs))) {
 if (bdrv_can_snapshot(bs) &&
-bdrv_snapshot_find(bs, snapshot, name) >= 0)
+bdrv_snapshot_find(bs, snapshot, name) == 0)
 {
 ret = bdrv_snapshot_delete(bs, name);
 if (ret < 0) {
@@ -1948,8 +1948,9 @@ int load_vmstate(const char *name)
 
 /* Don't even try to load empty VM states */
 ret = bdrv_snapshot_find(bs, &sn, name);
-if ((ret >= 0) && (sn.vm_state_size == 0))
-return -EINVAL;
+if ((ret == 0) && (sn.vm_state_size == 0)) {
+return -EINVAL;
+}
 
 /* restore the VM state */
 f = qemu_fopen_bdrv(bs, 0);
-- 
1.7.1




[Qemu-devel] [PATCH 0/3] savem: various cleanups

2010-07-28 Thread Miguel Di Ciurcio Filho
This series does some cleanups and changes how the monitor command 'savevm'
handles overwriting snapshots with existing names and makes 'savevm' always
create a name for an snapshot being taken.

I'm keeping track of some issues with savevm/loadvm on a wiki page [1].

All feedback is very appreciated.

Regards,

Miguel

[1] http://wiki.qemu.org/Features/SnapshottingImprovements

Miguel Di Ciurcio Filho (3):
  cleanup: bdrv_snaphost_find() returns zero or -ENOENT
  cleanup: del_existing_snapshots() must return the upstream error code
  savevm: prevent snapshot overwriting and generate a default name

 qemu-monitor.hx |9 ---
 savevm.c|   68 --
 2 files changed, 45 insertions(+), 32 deletions(-)




[Qemu-devel] [PATCH 2/3] cleanup: del_existing_snapshots() must return the upstream error code

2010-07-28 Thread Miguel Di Ciurcio Filho
Signed-off-by: Miguel Di Ciurcio Filho 
---
 savevm.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/savevm.c b/savevm.c
index 6c6adb0..fb38e8a 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1775,7 +1775,7 @@ static int del_existing_snapshots(Monitor *mon, const 
char *name)
 monitor_printf(mon,
"Error while deleting snapshot on '%s'\n",
bdrv_get_device_name(bs));
-return -1;
+return ret;
 }
 }
 }
-- 
1.7.1




[Qemu-devel] [PATCH 3/3] savevm: prevent snapshot overwriting and generate a default name

2010-07-28 Thread Miguel Di Ciurcio Filho
This patch address two issues.

1) When savevm is run using an previously saved snapshot id or name, it will
delete the original and create a new one, using the same id and name and not
prompting the user of what just happened.

This behaviour is not good, IMHO.

We add a '-f' parameter to savevm, to really force that to happen, in case the
user really wants to.

New behavior:
(qemu) savevm snap1
An snapshot named 'snap1' already exists

(qemu) savevm -f snap1

We do better error reporting in case '-f' is used too than before.

2) When savevm is run without a name or id, the name stays blank.

This is a first step to hide the internal id, because I don't see a reason to
expose this kind of internals to the user.

The new behavior is when savevm is run without parameters a name will be
created automaticaly, so the snapshot is accessible to the user without needing
the id when loadvm is run.

(qemu) savevm
(qemu) info snapshots
IDTAG VM SIZEDATE   VM CLOCK
1 vm-20100728134640  978K 2010-07-28 13:46:40   00:00:08.603

We use a name with the format 'vm-MMDDHHMMSS'.

TODO: I have no clue on how to create a timestamp string when using Windows.

Signed-off-by: Miguel Di Ciurcio Filho 
---
 qemu-monitor.hx |9 ---
 savevm.c|   59 --
 2 files changed, 40 insertions(+), 28 deletions(-)

diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 9c27b31..94e8178 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -275,14 +275,15 @@ ETEXI
 
 {
 .name   = "savevm",
-.args_type  = "name:s?",
-.params = "[tag|id]",
-.help   = "save a VM snapshot. If no tag or id are provided, a new 
snapshot is created",
+.args_type  = "force:-f,name:s?",
+.params = "[-f] [name]",
+.help   = "save a VM snapshot. If no name is provided, a new one 
will be generated"
+ "\n\t\t\t -f to overwrite an snapshot if it already 
exists",
 .mhandler.cmd = do_savevm,
 },
 
 STEXI
-...@item savevm [...@var{tag}|@var{id}]
+...@item savevm [-f] [...@var{tag}]
 @findex savevm
 Create a snapshot of the whole virtual machine. If @var{tag} is
 provided, it is used as human readable identifier. If there is already
diff --git a/savevm.c b/savevm.c
index fb38e8a..ae6f29f 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1785,7 +1785,7 @@ static int del_existing_snapshots(Monitor *mon, const 
char *name)
 
 void do_savevm(Monitor *mon, const QDict *qdict)
 {
-BlockDriverState *bs, *bs1;
+BlockDriverState *bs_vm_state, *bs;
 QEMUSnapshotInfo sn1, *sn = &sn1, old_sn1, *old_sn = &old_sn1;
 int ret;
 QEMUFile *f;
@@ -1796,7 +1796,9 @@ void do_savevm(Monitor *mon, const QDict *qdict)
 #else
 struct timeval tv;
 #endif
+struct tm tm;
 const char *name = qdict_get_try_str(qdict, "name");
+int force = qdict_get_try_bool(qdict, "force", 0);
 
 /* Verify if there is a device that doesn't support snapshots and is 
writable */
 bs = NULL;
@@ -1813,8 +1815,8 @@ void do_savevm(Monitor *mon, const QDict *qdict)
 }
 }
 
-bs = bdrv_snapshots();
-if (!bs) {
+bs_vm_state = bdrv_snapshots();
+if (!bs_vm_state) {
 monitor_printf(mon, "No block device can accept snapshots\n");
 return;
 }
@@ -1825,15 +1827,6 @@ void do_savevm(Monitor *mon, const QDict *qdict)
 vm_stop(0);
 
 memset(sn, 0, sizeof(*sn));
-if (name) {
-ret = bdrv_snapshot_find(bs, old_sn, name);
-if (ret >= 0) {
-pstrcpy(sn->name, sizeof(sn->name), old_sn->name);
-pstrcpy(sn->id_str, sizeof(sn->id_str), old_sn->id_str);
-} else {
-pstrcpy(sn->name, sizeof(sn->name), name);
-}
-}
 
 /* fill auxiliary fields */
 #ifdef _WIN32
@@ -1847,13 +1840,31 @@ void do_savevm(Monitor *mon, const QDict *qdict)
 #endif
 sn->vm_clock_nsec = qemu_get_clock(vm_clock);
 
-/* Delete old snapshots of the same name */
-if (name && del_existing_snapshots(mon, name) < 0) {
-goto the_end;
+if (name) {
+ret = bdrv_snapshot_find(bs_vm_state, old_sn, name);
+if (ret == 0) {
+if (force) {
+ret = del_existing_snapshots(mon, name);
+if (ret < 0) {
+monitor_printf(mon, "Error deleting snapshot '%s', error: 
%d\n",
+name, ret);
+goto the_end;
+}
+} else {
+monitor_printf(mon, "An snapshot named '%s' already exists\n", 
name);
+goto the_end;
+}
+}
+
+pstrcpy(sn->name, sizeof(sn->name), name);
+} else {
+/* TODO: handle Windows */
+localtime_r(&tv.tv_sec, &tm);
+strftime(sn->name, sizeof(sn->name), "vm-%Y%m%d%H%M%S", &tm);
 }
 
 /* save the VM state */
-f = qemu_fopen_bdrv(bs, 1);
+  

[Qemu-devel] Re: [RFC PATCH 08/14] KVM-test: Add a subtest of nic promisc

2010-07-28 Thread Lucas Meneghel Rodrigues
On Tue, 2010-07-20 at 09:35 +0800, Amos Kong wrote:
> This test mainly covers TCP sent from host to guest and from guest to host
> with repeatedly turn on/off NIC promiscuous mode.
> 
> Signed-off-by: Amos Kong 
> ---
>  0 files changed, 0 insertions(+), 0 deletions(-)
> 
> diff --git a/client/tests/kvm/tests/nic_promisc.py 
> b/client/tests/kvm/tests/nic_promisc.py
> new file mode 100644
> index 000..9a0c979
> --- /dev/null
> +++ b/client/tests/kvm/tests/nic_promisc.py
> @@ -0,0 +1,87 @@
> +import logging, commands
> +from autotest_lib.client.common_lib import error
> +import kvm_utils, kvm_test_utils, kvm_net_utils
> +
> +def run_nic_promisc(test, params, env):
> +"""
> +Test nic driver in promisc mode:
> +
> +1) Boot up a guest
> +2) Repeatedly enable/disable promiscuous mode in guest
> +3) TCP data transmission from host to guest, and from guest to host,
> +   with 1/1460/65000/1 bytes payloads
> +4) Clean temporary files
> +5) Stop enable/disable promiscuous mode change
> +
> +@param test: kvm test object
> +@param params: Dictionary with the test parameters
> +@param env: Dictionary with test environment
> +"""
> +timeout = int(params.get("login_timeout", 360))
> +vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
> +session = kvm_test_utils.wait_for_login(vm, timeout=timeout)
> +logging.info("Trying to log into guest '%s' by serial", vm.name)
> +session2 = kvm_utils.wait_for(lambda: vm.serial_login(),
> +  timeout, 0, step=2)
> +if not session2:
> +raise error.TestFail("Could not log into guest '%s'" % vm.name)
> +
> +def compare(filename):
> +cmd = "md5sum %s" % filename
> +s1, ret_host = commands.getstatusoutput(cmd)
> +s2, ret_guest = session.get_command_status_output(cmd)
> +if s1 != 0 or s2 != 0:
> +logging.debug("ret_host:%s, ret_guest:%s" % (ret_host, 
> ret_guest))
> +logging.error("Could not get md5, cmd:%s" % cmd)
> +return False
> +if ret_host.strip() != ret_guest.strip():
> +logging.debug("ret_host :%s, ret_guest:%s" % (ret_host, 
> ret_guest))
> +logging.error("Files' md5sum mismatch" % (receiver))
> +return False

^ The above debug messages will be confusing when looked by someone who
is not familiar with the test code, so we should make their lives
easier:

def compare(filename):
cmd = "md5sum %s" % filename
rc_host, md5_host = commands.getstatusoutput(cmd)
rc_guest, md5_guest = session.get_command_status_output(cmd)
if rc_host:
logging.debug('Could not get MD5 hash for file %s on host, output: 
%s', filename, md5_host)
return False
if rc_guest:
logging.debug('Could not get MD5 hash for file %s on guest, output: 
%s', filename, md5_guest)
return False
md5host = md5host.strip()
md5guest = md5guest.strip()
if md5host != md5guest:
logging.error('MD5 hash mismatch between file %s present on guest 
and on host', filename)
logging.error('MD5 hash for file on guest: %s, MD5 hash for file on 
host: %s', md5_host, md5_guest)
return False
return True


> +return True
> +
> +ethname = kvm_net_utils.get_linux_ifname(session, vm.get_macaddr(0))
> +set_promisc_cmd = "ip link set %s promisc on; sleep 0.01;" % ethname
> +set_promisc_cmd += "ip link set %s promisc off; sleep 0.01" % ethname

^ You could do the above on a single attribution, see comment on patch 7
of the patchseries.

> +logging.info("Set promisc change repeatedly in guest")
> +session2.sendline("while true; do %s; done" % set_promisc_cmd)
> +
> +dd_cmd = "dd if=/dev/urandom of=%s bs=%d count=1"
> +filename = "/tmp/nic_promisc_file"
> +file_size = params.get("file_size", "1, 1460, 65000, 
> 1").split(",")
> +try:
> +for size in file_size:
> +logging.info("Create %s bytes file on host" % size)
> +s, o = commands.getstatusoutput(dd_cmd % (filename, int(size)))
> +if s != 0:
> +logging.debug("Output: %s"% o)
> +raise error.TestFail("Create file on host failed")
> +
> +logging.info("Transfer file from host to guest")
> +if not vm.copy_files_to(filename, filename):
> +raise error.TestFail("File transfer failed")
> +if not compare(filename):
> +raise error.TestFail("Compare file failed")

^ It'd be better if we don't abruptly fail the whole test if we get a
failure for a single size, what about having a global failure counter,
and increment it if we have failures, making sure we log errors
appropriately?

> +logging.info("Create %s bytes file on guest" % size)
> +if session.get_command_status(dd_cmd % (filename, int(size))

Re: [Qemu-devel] [RFC PATCH 09/14] KVM-test: Add a subtest of multicast

2010-07-28 Thread Lucas Meneghel Rodrigues
On Tue, 2010-07-20 at 09:36 +0800, Amos Kong wrote:
> Use 'ping' to test send/recive multicat packets. Flood ping test is also 
> added.
> Limit guest network as 'bridge' mode, because multicast packets could not be
> transmitted to guest when using 'user' network.
> Add join_mcast.py for joining machine into multicast groups.
> 
> Signed-off-by: Amos Kong 
> ---
>  0 files changed, 0 insertions(+), 0 deletions(-)
> 
> diff --git a/client/tests/kvm/scripts/join_mcast.py 
> b/client/tests/kvm/scripts/join_mcast.py
> new file mode 100755
> index 000..0d90e5c
> --- /dev/null
> +++ b/client/tests/kvm/scripts/join_mcast.py
> @@ -0,0 +1,29 @@
> +import socket, struct, os, signal, sys
> +# this script is used to join machine into multicast groups
> +# author: Amos Kong 
> +
> +if len(sys.argv) < 4:
> +print """%s [mgroup_count] [prefix] [suffix]
> +mgroup_count: count of multicast addresses
> +prefix: multicast address prefix
> +suffix: multicast address suffix""" % sys.argv[0]
> +sys.exit()
> +
> +mgroup_count = int(sys.argv[1])
> +prefix = sys.argv[2]
> +suffix = int(sys.argv[3])
> +
> +s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
> +for i in range(mgroup_count):
> +mcast = prefix + "." + str(suffix + i)
> +try:
> +mreq = struct.pack("4sl", socket.inet_aton(mcast), socket.INADDR_ANY)
> +s.setsockopt(socket.IPPROTO_IP, socket.IP_ADD_MEMBERSHIP, mreq)
> +except:
> +s.close()
> +print "Could not join multicast: %s" % mcast
> +raise
> +
> +print "join_mcast_pid:%s" % os.getpid()
> +os.kill(os.getpid(), signal.SIGSTOP)
> +s.close()
> diff --git a/client/tests/kvm/tests/multicast.py 
> b/client/tests/kvm/tests/multicast.py
> new file mode 100644
> index 000..6b0e106
> --- /dev/null
> +++ b/client/tests/kvm/tests/multicast.py
> @@ -0,0 +1,67 @@
> +import logging, commands, os, re
> +from autotest_lib.client.common_lib import error
> +import kvm_test_utils, kvm_net_utils
> +
> +
> +def run_multicast(test, params, env):
> +"""
> +Test multicast function of nic (rtl8139/e1000/virtio)
> +
> +1) Create a VM
> +2) Join guest into multicast groups
> +3) Ping multicast addresses on host
> +4) Flood ping test with different size of packets
> +5) Final ping test and check if lose packet
> +
> +@param test: Kvm test object
> +@param params: Dictionary with the test parameters.
> +@param env: Dictionary with test environment.
> +"""
> +vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
> +session = kvm_test_utils.wait_for_login(vm,
> +  timeout=int(params.get("login_timeout", 360)))
> +
> +# stop iptable/selinux on guest/host
> +cmd = "/etc/init.d/iptables stop && echo 0 > /selinux/enforce"

^ Different linux distros handle init scripts differently, so it's
better to just flush the firewall rules with iptables -F instead of
resorting to the init script.

Also, not all distros use selinux, so it'd be better to test for the
presence of selinux on the system (for example, by checking if there's
a /selinux/enforce file before trying to write to it). Also, need to do
proper return code checking/treatment.

> +session.get_command_status(cmd)
> +commands.getoutput(cmd)
> +# make sure guest replies to broadcasts
> +session.get_command_status("echo 0 > /proc/sys/net/ipv4/icmp_echo_ignore"
> +"_broadcasts && echo 0 > 
> /proc/sys/net/ipv4/icmp_echo_ignore_all")
> +
> +# base multicast address
> +mcast = params.get("mcast", "225.0.0.1")
> +# count of multicast addresses, less than 20
> +mgroup_count = int(params.get("mgroup_count", 5))
> +flood_minutes = float(params.get("flood_minutes", 10))
> +ifname = vm.get_ifname()
> +prefix = re.findall("\d+.\d+.\d+", mcast)[0]
> +suffix = int(re.findall("\d+", mcast)[-1])
> +# copy python script to guest for joining guest to multicast groups
> +mcast_path = os.path.join(test.bindir, "scripts/join_mcast.py")
> +vm.copy_files_to(mcast_path, "/tmp")

^ What if copy_files_to fails? Need to do proper return handling here

> +output = session.get_command_output("python /tmp/join_mcast.py %d %s %d" 
> %
> +(mgroup_count, prefix, suffix))
> +# if success to join multicast the process will be paused, and return 
> pid.
> +if not re.findall("join_mcast_pid:(\d+)", output):
> +raise error.TestFail("Can't join multicast groups,output:%s" % 
> output)
> +pid = output.split()[0]
> +
> +try:
> +for i in range(mgroup_count):
> +new_suffix = suffix + i
> +mcast = "%s.%d" % (prefix, new_suffix)
> +logging.info("Initial ping test, mcast: %s", mcast)
> +s, o = kvm_net_utils.ping(mcast, 10, interface=ifname, 
> timeout=20)
> +if s != 0:
> +raise error.TestFail(" Ping return non-zero value %s" % o)
> +logging.info("Flo

Re: [Qemu-devel] [PATCH 11/12] linux-user: Extract load_elf_image from load_elf_interp.

2010-07-28 Thread Edgar E. Iglesias
On Tue, Jul 27, 2010 at 10:25:37AM -0700, Richard Henderson wrote:
> Moving toward a single copy of the elf binary loading code.
> Fill in the details of the loaded image into a struct image_info.
> 
> Adjust create_elf_tables to read from such structures instead
> of from a collection of passed arguments.  Don't return error
> values from load_elf_interp; always exit(-1) with a message to
> stderr.  Collect elf_interpreter handling in load_elf_binary
> to a common spot.
> 
> Signed-off-by: Richard Henderson 
> ---
>  linux-user/elfload.c |  341 -
>  1 files changed, 167 insertions(+), 174 deletions(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 61167cd..8ff9b6a 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -1079,11 +1079,9 @@ static void zero_bss(abi_ulong elf_bss, abi_ulong 
> last_bss, int prot)
>  }
>  
>  static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc,
> -   struct elfhdr * exec,
> -   abi_ulong load_addr,
> -   abi_ulong load_bias,
> -   abi_ulong interp_load_addr,
> -   struct image_info *info)
> +   struct elfhdr *exec,
> +   struct image_info *info,
> +   struct image_info *interp_info)
>  {
>  abi_ulong sp;
>  int size;
> @@ -1128,13 +1126,13 @@ static abi_ulong create_elf_tables(abi_ulong p, int 
> argc, int envc,
>  NEW_AUX_ENT (AT_NULL, 0);
>  
>  /* There must be exactly DLINFO_ITEMS entries here.  */
> -NEW_AUX_ENT(AT_PHDR, (abi_ulong)(load_addr + exec->e_phoff));
> +NEW_AUX_ENT(AT_PHDR, (abi_ulong)(info->load_addr + exec->e_phoff));
>  NEW_AUX_ENT(AT_PHENT, (abi_ulong)(sizeof (struct elf_phdr)));
>  NEW_AUX_ENT(AT_PHNUM, (abi_ulong)(exec->e_phnum));
>  NEW_AUX_ENT(AT_PAGESZ, (abi_ulong)(TARGET_PAGE_SIZE));
> -NEW_AUX_ENT(AT_BASE, (abi_ulong)(interp_load_addr));
> +NEW_AUX_ENT(AT_BASE, (abi_ulong)(interp_info->load_addr));


Hi Richard,

I think this part breaks loading of statically linked ELFs (no
interpreter). I beleive Linux sets AT_BASE to zero in those cases.

Cheers



Re: [Qemu-devel] [RFC PATCH 10/14] KVM-test: Add a subtest of pxe

2010-07-28 Thread Lucas Meneghel Rodrigues
On Tue, 2010-07-20 at 09:36 +0800, Amos Kong wrote:
> This case just snoop tftp packet through tcpdump, it depends on public dhcp
> server, better to test it through dnsmasq.

It would be a good idea to have an alternate implementation using
dnsmasq, but not urgent.

> Signed-off-by: Jason Wang 
> Signed-off-by: Amos Kong 
> ---
>  0 files changed, 0 insertions(+), 0 deletions(-)
> 
> diff --git a/client/tests/kvm/tests/pxe.py b/client/tests/kvm/tests/pxe.py
> new file mode 100644
> index 000..8859aaa
> --- /dev/null
> +++ b/client/tests/kvm/tests/pxe.py
> @@ -0,0 +1,30 @@
> +import logging
> +from autotest_lib.client.common_lib import error
> +import kvm_subprocess, kvm_test_utils, kvm_utils
> +
> +
> +def run_pxe(test, params, env):
> +"""
> +PXE test:
> +
> +1) Snoop the tftp packet in the tap device
> +2) Wait for some seconds
> +3) Check whether capture tftp packets
> +
> +@param test: kvm test object
> +@param params: Dictionary with the test parameters
> +@param env: Dictionary with test environment.
> +"""
> +vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
> +timeout = int(params.get("pxe_timeout", 60))
> +
> +logging.info("Try to boot from pxe")
> +status, output = kvm_subprocess.run_fg("tcpdump -nli %s" % 
> vm.get_ifname(),
> +   logging.debug,
> +   "(pxe) ",
> +   timeout)

^ The only complaint I could make here is that since this command
doesn't need to live throughout tests, utils.run would do just fine.
Other than that, looks fine to me.

> +logging.info("Analysing the tcpdump result...")

^ typo, analyzing 

> +if not "tftp" in output:
> +raise error.TestFail("Couldn't find tftp packet in %s seconds" % 
> timeout)
> +logging.info("Found tftp packet")
> diff --git a/client/tests/kvm/tests_base.cfg.sample 
> b/client/tests/kvm/tests_base.cfg.sample
> index 9594a38..5515601 100644
> --- a/client/tests/kvm/tests_base.cfg.sample
> +++ b/client/tests/kvm/tests_base.cfg.sample
> @@ -381,6 +381,19 @@ variants:
>  mgroup_count = 20
>  flood_minutes = 1
>  
> +- pxe:
> +type = pxe
> +images = pxe
> +image_name_pxe = pxe-test
> +image_size_pxe = 1G
> +force_create_image_pxe = yes
> +remove_image_pxe = yes
> +extra_params += ' -boot n'
> +kill_vm_on_error = yes
> +network = bridge
> +restart_vm = yes
> +pxe_timeout = 60
> +
>  - physical_resources_check: install setup unattended_install.cdrom
>  type = physical_resources_check
>  catch_uuid_cmd = dmidecode | awk -F: '/UUID/ {print $2}'
> 
> 





Re: [Qemu-devel] [PATCH 07/12] linux-user: Load symbols from the interpreter.

2010-07-28 Thread Edgar E. Iglesias
On Tue, Jul 27, 2010 at 10:25:33AM -0700, Richard Henderson wrote:
> First, adjust load_symbols to accept a load_bias parameter.  At the same
> time, read the entire section header table in one go, use pread instead
> f lseek+read for the symbol and string tables, and properly free
> allocated structures on error exit paths.
> 
> Second, adjust load_elf_interp to compute load_bias.  This requires
> finding out the built-in load addresses.  Which allows us to honor a
> pre-linked interpreter image when possible, and eliminate the hard-coded
> INTERP_MAP_SIZE value.


This was neat, I wish we've had this feature before :)

Cheers



> 
> Signed-off-by: Richard Henderson 
> ---
>  linux-user/elfload.c |  189 
> +++---
>  1 files changed, 101 insertions(+), 88 deletions(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 3cbb1f4..6b57a91 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -829,9 +829,6 @@ struct exec
>  #define ZMAGIC 0413
>  #define QMAGIC 0314
>  
> -/* max code+data+bss space allocated to elf interpreter */
> -#define INTERP_MAP_SIZE (32 * 1024 * 1024)
> -
>  /* max code+data+bss+brk space allocated to ET_DYN executables */
>  #define ET_DYN_MAP_SIZE (128 * 1024 * 1024)
>  
> @@ -920,6 +917,7 @@ static inline void bswap_sym(struct elf_sym *sym) { }
>  #ifdef USE_ELF_CORE_DUMP
>  static int elf_core_dump(int, const CPUState *);
>  #endif /* USE_ELF_CORE_DUMP */
> +static void load_symbols(struct elfhdr *hdr, int fd, abi_ulong load_bias);
>  
>  /*
>   * 'copy_elf_strings()' copies argument/envelope strings from user
> @@ -1146,15 +1144,11 @@ static abi_ulong load_elf_interp(struct elfhdr * 
> interp_elf_ex,
>   char bprm_buf[BPRM_BUF_SIZE])
>  {
>  struct elf_phdr *elf_phdata  =  NULL;
> -struct elf_phdr *eppnt;
> -abi_ulong load_addr = 0;
> -int load_addr_set = 0;
> +abi_ulong load_addr, load_bias, loaddr, hiaddr;
>  int retval;
>  abi_ulong error;
>  int i;
>  
> -error = 0;
> -
>  bswap_ehdr(interp_elf_ex);
>  /* First of all, some simple consistency checks */
>  if ((interp_elf_ex->e_type != ET_EXEC &&
> @@ -1163,7 +1157,6 @@ static abi_ulong load_elf_interp(struct elfhdr * 
> interp_elf_ex,
>  return ~((abi_ulong)0UL);
>  }
>  
> -
>  /* Now read in all of the header information */
>  
>  if (sizeof(struct elf_phdr) * interp_elf_ex->e_phnum > TARGET_PAGE_SIZE)
> @@ -1196,41 +1189,56 @@ static abi_ulong load_elf_interp(struct elfhdr * 
> interp_elf_ex,
>  }
>  bswap_phdr(elf_phdata, interp_elf_ex->e_phnum);
>  
> +/* Find the maximum size of the image and allocate an appropriate
> +   amount of memory to handle that.  */
> +loaddr = -1, hiaddr = 0;
> +for (i = 0; i < interp_elf_ex->e_phnum; ++i) {
> +if (elf_phdata[i].p_type == PT_LOAD) {
> +abi_ulong a = elf_phdata[i].p_vaddr;
> +if (a < loaddr) {
> +loaddr = a;
> +}
> +a += elf_phdata[i].p_memsz;
> +if (a > hiaddr) {
> +hiaddr = a;
> +}
> +}
> +}
> +
> +load_addr = loaddr;
>  if (interp_elf_ex->e_type == ET_DYN) {
> -/* in order to avoid hardcoding the interpreter load
> -   address in qemu, we allocate a big enough memory zone */
> -error = target_mmap(0, INTERP_MAP_SIZE,
> -PROT_NONE, MAP_PRIVATE | MAP_ANON,
> --1, 0);
> -if (error == -1) {
> +/* The image indicates that it can be loaded anywhere.  Find a
> +   location that can hold the memory space required.  If the
> +   image is pre-linked, LOADDR will be non-zero.  Since we do
> +   not supply MAP_FIXED here we'll use that address if and
> +   only if it remains available.  */
> +load_addr = target_mmap(loaddr, hiaddr - loaddr, PROT_NONE,
> +MAP_PRIVATE | MAP_ANON | MAP_NORESERVE,
> +-1, 0);
> +if (load_addr == -1) {
>  perror("mmap");
>  exit(-1);
>  }
> -load_addr = error;
> -load_addr_set = 1;
>  }
> +load_bias = load_addr - loaddr;
>  
> -eppnt = elf_phdata;
> -for(i=0; ie_phnum; i++, eppnt++)
> +for (i = 0; i < interp_elf_ex->e_phnum; i++) {
> +struct elf_phdr *eppnt = elf_phdata + i;
>  if (eppnt->p_type == PT_LOAD) {
> -int elf_type = MAP_PRIVATE | MAP_DENYWRITE;
> +abi_ulong vaddr, vaddr_po, vaddr_ps, vaddr_ef, vaddr_em;
>  int elf_prot = 0;
> -abi_ulong vaddr = 0;
>  
>  if (eppnt->p_flags & PF_R) elf_prot =  PROT_READ;
>  if (eppnt->p_flags & PF_W) elf_prot |= PROT_WRITE;
>  if (eppnt->p_flags & PF_X) elf_prot |= PROT_EXEC;
> -if (interp_elf_ex->e_type == 

Re: [Qemu-devel] [RFC PATCH 11/14] KVM-test: Add a subtest of changing mac address

2010-07-28 Thread Lucas Meneghel Rodrigues
On Tue, 2010-07-20 at 09:36 +0800, Amos Kong wrote:
> Mainly test steps:
> 1. get a new mac from pool, and the old mac addr of guest.
> 2. execute the mac_change.sh in guest.
> 3. relogin to guest and query the interfaces info by `ifconfig`
> 
> Signed-off-by: Cao, Chen 
> Signed-off-by: Amos Kong 
> ---
>  0 files changed, 0 insertions(+), 0 deletions(-)
> 
> diff --git a/client/tests/kvm/tests/mac_change.py 
> b/client/tests/kvm/tests/mac_change.py
> new file mode 100644
> index 000..dc93377
> --- /dev/null
> +++ b/client/tests/kvm/tests/mac_change.py
> @@ -0,0 +1,66 @@
> +import logging
> +from autotest_lib.client.common_lib import error
> +import kvm_utils, kvm_test_utils, kvm_net_utils
> +
> +
> +def run_mac_change(test, params, env):
> +"""
> +Change MAC Address of Guest.
> +
> +1. get a new mac from pool, and the old mac addr of guest.
> +2. set new mac in guest and regain new IP.
> +3. re-log into guest with new mac
> +
> +@param test: kvm test object
> +@param params: Dictionary with the test parameters
> +@param env: Dictionary with test environment.
> +"""
> +timeout = int(params.get("login_timeout", 360))
> +vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
> +logging.info("Trying to log into guest '%s' by serial", vm.name)
> +session = kvm_utils.wait_for(lambda: vm.serial_login(),
> +  timeout, 0, step=2)

^ One thing that I forgot to comment on previous patches: For more
clarity, it'd be good to name the session variable in a way that lets
people refer easily that is a serial session

session_serial = ...

> +if not session:
> +raise error.TestFail("Could not log into guest '%s'" % vm.name)
> +
> +old_mac = vm.get_macaddr(0)
> +kvm_utils.put_mac_to_pool(vm.root_dir, old_mac, vm.instance)
> +new_mac = kvm_utils.get_mac_from_pool(vm.root_dir,
> +  vm=vm.instance,
> +  nic_index=0,
> +  prefix=vm.mac_prefix)
> +logging.info("The initial MAC address is %s" % old_mac)
> +interface = kvm_net_utils.get_linux_ifname(session, old_mac)
> +
> +# Start change mac address
> +logging.info("Changing mac address to %s" % new_mac)
> +change_cmd = "ifconfig %s down && ifconfig %s hw ether %s && ifconfig %s 
> up"\
> + % (interface, interface, new_mac, interface)
> +if session.get_command_status(change_cmd) != 0:
> +raise error.TestFail("Fail to send mac_change command")
> +
> +# Verify whether mac address is changed to new one
> +logging.info("Verifying the new mac address")
> +if session.get_command_status("ifconfig | grep -i %s" % new_mac) != 0:
> +raise error.TestFail("Fail to change mac address")
> +
> +# Restart `dhclient' to regain IP for new mac address
> +logging.info("Re-start the network to gain new ip")
> +dhclient_cmd = "dhclient -r && dhclient %s" % interface
> +session.sendline(dhclient_cmd)
> +
> +# Re-log into the guest after changing mac address
> +if kvm_utils.wait_for(session.is_responsive, 120, 20, 3):
> +# Just warning when failed to see the session become dead,
> +# because there is a little chance the ip does not change.
> +logging.warn("The session is still responsive, settings may fail.")

^ Isn't this a serial session? Then why the IP of guest changing would
make this session un-responsive? I think the best idea here is to:

1) Release the IP through dhclient -r [interface]
2) Make sure we can't stablish a ssh based session to the guest by
making a try/except block with kvm_test_utils.wait_for_login() with the
appropriate timeouts and other parameters, if succeeds, fail the test,
if it doesn't, proceed with the test.
3) Get a new IP with dhclient [interface]
4) Try to stablish a new, ssh based session to the guest and see if that
works.

> +session.close()
> +
> +# Re-log into guest and check if session is responsive
> +logging.info("Re-log into the guest")
> +session = kvm_test_utils.wait_for_login(vm,
> +  timeout=int(params.get("login_timeout", 360)))
> +if not session.is_responsive():
> +raise error.TestFail("The new session is not responsive.")

^ Is it possible that right after you stablish the session it becomes
non-responsive? It seems like a redundant verification step to me.

> +session.close()
> diff --git a/client/tests/kvm/tests_base.cfg.sample 
> b/client/tests/kvm/tests_base.cfg.sample
> index 5515601..7716d48 100644
> --- a/client/tests/kvm/tests_base.cfg.sample
> +++ b/client/tests/kvm/tests_base.cfg.sample
> @@ -394,6 +394,10 @@ variants:
>  restart_vm = yes
>  pxe_timeout = 60
>  
> +- mac_change: install setup unattended_install.cdrom
> +type = mac_change
> +kill_vm = yes
> +
>  - physical_resources_check: install setup unattende

[Qemu-devel] [PATCH 1/1] Remove unused eventfd.h

2010-07-28 Thread Mike McCormack
This header is not present on my system and causes a build
failure, but is also not used in these files, so remove it.

Signed-off-by: Mike McCormack 
---
 hw/vhost.c |1 -
 hw/vhost_net.c |1 -
 2 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/hw/vhost.c b/hw/vhost.c
index 65709d0..34c4745 100644
--- a/hw/vhost.c
+++ b/hw/vhost.c
@@ -11,7 +11,6 @@
  */
 
 #include 
-#include 
 #include "vhost.h"
 #include "hw/hw.h"
 /* For range_get_last */
diff --git a/hw/vhost_net.c b/hw/vhost_net.c
index 606aa0c..0c00de2 100644
--- a/hw/vhost_net.c
+++ b/hw/vhost_net.c
@@ -20,7 +20,6 @@
 
 #ifdef CONFIG_VHOST_NET
 #include 
-#include 
 #include 
 #include 
 #include 
-- 
1.5.6.5




Re: [Qemu-devel] [PATCH 11/12] linux-user: Extract load_elf_image from load_elf_interp.

2010-07-28 Thread Richard Henderson
On 07/28/2010 03:00 PM, Edgar E. Iglesias wrote:
>> -NEW_AUX_ENT(AT_BASE, (abi_ulong)(interp_load_addr));
>> +NEW_AUX_ENT(AT_BASE, (abi_ulong)(interp_info->load_addr));
> 
> 
> Hi Richard,
> 
> I think this part breaks loading of statically linked ELFs (no
> interpreter). I beleive Linux sets AT_BASE to zero in those cases.

You're right.  This should be 

  interp_info ? interp_info->load_addr : 0


r~



[Qemu-devel] [Bug 595906] Re: [ARM] All variants of ADDSUBX, SUBADDX give incorrect results

2010-07-28 Thread cmchao
The patch has been included in v0.12.5

** Changed in: qemu
   Status: In Progress => Fix Released

-- 
[ARM] All variants of ADDSUBX,SUBADDX give incorrect results
https://bugs.launchpad.net/bugs/595906
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.

Status in QEMU: Fix Released

Bug description:
All variants of the ADDSUBX/SUBADDX instructions seem to be implemented 
incorrectly, i.e.

MOV r12, #0
LDR r0, =0x18004800
LDR r1, =0x30006000
QADDSUBX r12, r0, r1; Should give 0x78001800 - gives 0x4800e800

This happens with latest git HEAD.





[Qemu-devel] [Bug 591320] Re: [ARM]: SIMD add/sub instructions are incorrect

2010-07-28 Thread cmchao
The patch has been included in v0.12.5

** Changed in: qemu
   Status: New => Fix Released

-- 
[ARM]: SIMD add/sub instructions are incorrect
https://bugs.launchpad.net/bugs/591320
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.

Status in QEMU: Fix Released

Bug description:
The thumb2 and unsigned arm state SIMD add/sub instructions are implemented 
incorrectly, for example:

  UQSUB8 r0, r1, r0

gives r0 as 0,  where r0 is 0x12345678 and r1 is 0x23456789 in ARM state, and:

  UHSUB8 r0, r1, r0

gives r0 as 0xbe01, where r0 is 0x12345678 and r1 is 0x23456789 in thumb2 
state.

This problem is present in git HEAD, (at time of writing, 
fd1dc858370d9a9ac7ea2512812c3a152ee6484b).





[Qemu-devel] [Bug 611142] [NEW] seabios should have native scsi support

2010-07-28 Thread Scott Moser
Public bug reported:

Binary package hint: seabios

Currently when a grub multiboot image is booted with 'kvm -kernel' and
'biosdisk' module, it will see block devices of type IDE or virtio.  It
will not see scsi devices.

To demonstrate this:
$ qemu-img create -f qcow2 disk.img 1G
$ grub-mkrescue --output=rescue.iso
$ grub-mkimage -O i386-pc --output=grub-mb.img biosdisk minicmd part_msdos

An 'ls' inside the grub prompt will show hard disks (hd0) on if:
a.) -drive uses interface of virtio or scsi
or
b.) kvm boots boot from a cdrom or floppy 

For example, with these commands, grub will see a '(hd0)'
$ kvm -drive file=disk.img,if=scsi,boot=on -cdrom rescue.iso -boot d
$ kvm -drive file=disk.img,if=scsi,boot=on -floppy rescue.iso -boot a
$ kvm -drive file=disk.img,if=virtio,boot=on -cdrom rescue.iso -boot d
$ kvm -drive file=disk.img,if=ide,boot=on -cdrom rescue.iso -boot d
$ kvm -drive file=disk.img,if=virtio,boot=on -kernel grub-mb.img 
$ kvm -drive file=disk.img,if=ide,boot=on -kernel grub-mb.img 

But the following will not:
$ kvm -drive file=disk.img,if=scsi,boot=on -kernel grub-mb.img

ProblemType: Bug
DistroRelease: Ubuntu 10.10
Package: seabios 0.6.0-0ubuntu1
ProcVersionSignature: User Name 2.6.32-305.9-ec2 2.6.32.11+drm33.2
Uname: Linux 2.6.32-305-ec2 i686
Architecture: i386
Date: Thu Jul 29 03:21:21 2010
Dependencies:
 
Ec2AMI: ami-e930db80
Ec2AMIManifest: 
ubuntu-images-testing-us/ubuntu-maverick-daily-i386-server-20100727.manifest.xml
Ec2AvailabilityZone: us-east-1b
Ec2InstanceType: m1.small
Ec2Kernel: aki-407d9529
Ec2Ramdisk: unavailable
PackageArchitecture: all
ProcEnviron:
 PATH=(custom, user)
 LANG=en_US.UTF-8
 SHELL=/bin/bash
SourcePackage: seabios

** Affects: qemu
 Importance: Undecided
 Status: New

** Affects: qemu-kvm (Ubuntu)
 Importance: Undecided
 Status: New

** Affects: seabios (Ubuntu)
 Importance: Undecided
 Status: New


** Tags: apport-bug ec2-images i386 maverick

** Also affects: qemu-kvm (Ubuntu)
   Importance: Undecided
   Status: New

** Also affects: qemu
   Importance: Undecided
   Status: New

-- 
seabios should have native scsi support
https://bugs.launchpad.net/bugs/611142
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.

Status in QEMU: New
Status in “qemu-kvm” package in Ubuntu: New
Status in “seabios” package in Ubuntu: New

Bug description:
Binary package hint: seabios

Currently when a grub multiboot image is booted with 'kvm -kernel' and 
'biosdisk' module, it will see block devices of type IDE or virtio.  It will 
not see scsi devices.

To demonstrate this:
$ qemu-img create -f qcow2 disk.img 1G
$ grub-mkrescue --output=rescue.iso
$ grub-mkimage -O i386-pc --output=grub-mb.img biosdisk minicmd part_msdos

An 'ls' inside the grub prompt will show hard disks (hd0) on if:
a.) -drive uses interface of virtio or scsi
or
b.) kvm boots boot from a cdrom or floppy 

For example, with these commands, grub will see a '(hd0)'
$ kvm -drive file=disk.img,if=scsi,boot=on -cdrom rescue.iso -boot d
$ kvm -drive file=disk.img,if=scsi,boot=on -floppy rescue.iso -boot a
$ kvm -drive file=disk.img,if=virtio,boot=on -cdrom rescue.iso -boot d
$ kvm -drive file=disk.img,if=ide,boot=on -cdrom rescue.iso -boot d
$ kvm -drive file=disk.img,if=virtio,boot=on -kernel grub-mb.img 
$ kvm -drive file=disk.img,if=ide,boot=on -kernel grub-mb.img 

But the following will not:
$ kvm -drive file=disk.img,if=scsi,boot=on -kernel grub-mb.img

ProblemType: Bug
DistroRelease: Ubuntu 10.10
Package: seabios 0.6.0-0ubuntu1
ProcVersionSignature: User Name 2.6.32-305.9-ec2 2.6.32.11+drm33.2
Uname: Linux 2.6.32-305-ec2 i686
Architecture: i386
Date: Thu Jul 29 03:21:21 2010
Dependencies:
 
Ec2AMI: ami-e930db80
Ec2AMIManifest: 
ubuntu-images-testing-us/ubuntu-maverick-daily-i386-server-20100727.manifest.xml
Ec2AvailabilityZone: us-east-1b
Ec2InstanceType: m1.small
Ec2Kernel: aki-407d9529
Ec2Ramdisk: unavailable
PackageArchitecture: all
ProcEnviron:
 PATH=(custom, user)
 LANG=en_US.UTF-8
 SHELL=/bin/bash
SourcePackage: seabios





[Qemu-devel] [Bug 611142] Re: seabios should have native scsi support

2010-07-28 Thread Scott Moser
I'm fully aware that this isn't a high priority bug for anyone primarily due to 
the fact that it uses the word 'scsi'.
However, I wanted to document it here.

Also, I'm attaching an IRC log of a conversation with aligouri in #kvm
discussing the issue.


** Attachment added: "#kvm discussion about this issue"
   http://launchpadlibrarian.net/52670007/aligouri-%23kvm.txt

-- 
seabios should have native scsi support
https://bugs.launchpad.net/bugs/611142
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.

Status in QEMU: New
Status in “qemu-kvm” package in Ubuntu: New
Status in “seabios” package in Ubuntu: New

Bug description:
Binary package hint: seabios

Currently when a grub multiboot image is booted with 'kvm -kernel' and 
'biosdisk' module, it will see block devices of type IDE or virtio.  It will 
not see scsi devices.

To demonstrate this:
$ qemu-img create -f qcow2 disk.img 1G
$ grub-mkrescue --output=rescue.iso
$ grub-mkimage -O i386-pc --output=grub-mb.img biosdisk minicmd part_msdos

An 'ls' inside the grub prompt will show hard disks (hd0) on if:
a.) -drive uses interface of virtio or scsi
or
b.) kvm boots boot from a cdrom or floppy 

For example, with these commands, grub will see a '(hd0)'
$ kvm -drive file=disk.img,if=scsi,boot=on -cdrom rescue.iso -boot d
$ kvm -drive file=disk.img,if=scsi,boot=on -floppy rescue.iso -boot a
$ kvm -drive file=disk.img,if=virtio,boot=on -cdrom rescue.iso -boot d
$ kvm -drive file=disk.img,if=ide,boot=on -cdrom rescue.iso -boot d
$ kvm -drive file=disk.img,if=virtio,boot=on -kernel grub-mb.img 
$ kvm -drive file=disk.img,if=ide,boot=on -kernel grub-mb.img 

But the following will not:
$ kvm -drive file=disk.img,if=scsi,boot=on -kernel grub-mb.img

ProblemType: Bug
DistroRelease: Ubuntu 10.10
Package: seabios 0.6.0-0ubuntu1
ProcVersionSignature: User Name 2.6.32-305.9-ec2 2.6.32.11+drm33.2
Uname: Linux 2.6.32-305-ec2 i686
Architecture: i386
Date: Thu Jul 29 03:21:21 2010
Dependencies:
 
Ec2AMI: ami-e930db80
Ec2AMIManifest: 
ubuntu-images-testing-us/ubuntu-maverick-daily-i386-server-20100727.manifest.xml
Ec2AvailabilityZone: us-east-1b
Ec2InstanceType: m1.small
Ec2Kernel: aki-407d9529
Ec2Ramdisk: unavailable
PackageArchitecture: all
ProcEnviron:
 PATH=(custom, user)
 LANG=en_US.UTF-8
 SHELL=/bin/bash
SourcePackage: seabios