[PATCH 0/17][REVIEW] Backport candidates for stable-0.10

[PATCH 0/17][REVIEW] Backport candidates for stable-0.10

[Anthony Liguori]

This is a set of patches from master that I think are candidates for the
stable-0.10 branch.   They're all KVM specific.

I tried to be conservative and didn't include any ia64, PPC, or passthrough
fixes.  I don't know the state of those subsystems well enough to judge whether
the commits help or not.

To detect set of candidates to consider, I used the following git command:

 git log ^kvm-stable-start origin/master..kvm

This gives a range of commits that do not include anything reachable from
kvm-stable-start or origin/master (which is upstream QEMU) starting at the
master branch in kvm.

kvm-stable-start is a local tag to:

  commit 8d890e234cdf5860395c4cb93b831ae37aab41e9
  Author: Zhang, Yang <yang.zhang@intel.com>
  Date:   Thu Mar 5 15:10:22 2009 +0800

      kvm: external module: add kvm-ia64.c to hack files list

As best I can tell, the is the point where KVM's stable branch forked from the
master branch.  If anyone knows a better way to figure this out in git, please
let me know.

I've done some regression testing.  It looks like savevm/loadvm is broken in
stable-0.10 right now (KVM only) so that limits the testing I can do there.

One of the patches required enough fuzzing that I'd like to get an Ack on it.
I've marked it appropriately.

[PATCH 01/17] kvm: libkvm: protect irq injection status code with KVM_CAP_IRQ_INJECT_STATUS

[Anthony Liguori]

From: Gleb Natapov <gleb@redhat.com>

Fixes builds with older kernel headers.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/kvm/libkvm/libkvm.c b/kvm/libkvm/libkvm.c
index 0ac1c28..7c65f31 100644
--- a/kvm/libkvm/libkvm.c
+++ b/kvm/libkvm/libkvm.c
@@ -667,9 +667,14 @@ int kvm_set_irq_level(kvm_context_t kvm, int irq, int level, int *status)
 	if (r == -1)
 		perror("kvm_set_irq_level");
 
-	if (status)
+	if (status) {
+#ifdef KVM_CAP_IRQ_INJECT_STATUS
 		*status = (kvm->irqchip_inject_ioctl == KVM_IRQ_LINE) ?
 			1 : event.status;
+#else
+		*status = 1;
+#endif
+	}
 
 	return 1;
 }
-- 
1.6.0.6

Re: [PATCH 01/17] kvm: libkvm: protect irq injection status code with KVM_CAP_IRQ_INJECT_STATUS

[Avi Kivity]

Anthony Liguori wrote:
> From: Gleb Natapov <gleb@redhat.com>
>
> Fixes builds with older kernel headers.
>   

Since we build with a specific set of headers now (rather that 'make 
sync'), this isn't necessary.

-- 
error compiling committee.c: too many arguments to function

[PATCH 02/17] kvm: testsuite: compile fix - avoid raw string literal

[Anthony Liguori]

From: Jochen Roth <jroth@linux.vnet.ibm.com>

This patch fixes compilation problems of kvm-userspace on current gcc
4.4 compilers which implement the following standard:
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2442.htm

Signed-off-by: Jochen Roth <jroth@linux.vnet.ibm.com>
Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/kvm/user/test/x86/apic.c b/kvm/user/test/x86/apic.c
index 9c6205b..7794615 100644
--- a/kvm/user/test/x86/apic.c
+++ b/kvm/user/test/x86/apic.c
@@ -54,14 +54,14 @@ asm (
     "push %r9  \n\t"
     "push %r8  \n\t"
 #endif
-    "push %"R"di \n\t"
-    "push %"R"si \n\t"
-    "push %"R"bp \n\t"
-    "push %"R"sp \n\t"
-    "push %"R"bx \n\t"
-    "push %"R"dx \n\t"
-    "push %"R"cx \n\t"
-    "push %"R"ax \n\t"
+    "push %"R "di \n\t"
+    "push %"R "si \n\t"
+    "push %"R "bp \n\t"
+    "push %"R "sp \n\t"
+    "push %"R "bx \n\t"
+    "push %"R "dx \n\t"
+    "push %"R "cx \n\t"
+    "push %"R "ax \n\t"
 #ifdef __x86_64__
     "mov %rsp, %rdi \n\t"
     "callq *8*16(%rsp) \n\t"
@@ -70,14 +70,14 @@ asm (
     "calll *4+4*8(%esp) \n\t"
     "add $4, %esp \n\t"
 #endif
-    "pop %"R"ax \n\t"
-    "pop %"R"cx \n\t"
-    "pop %"R"dx \n\t"
-    "pop %"R"bx \n\t"
-    "pop %"R"bp \n\t"
-    "pop %"R"bp \n\t"
-    "pop %"R"si \n\t"
-    "pop %"R"di \n\t"
+    "pop %"R "ax \n\t"
+    "pop %"R "cx \n\t"
+    "pop %"R "dx \n\t"
+    "pop %"R "bx \n\t"
+    "pop %"R "bp \n\t"
+    "pop %"R "bp \n\t"
+    "pop %"R "si \n\t"
+    "pop %"R "di \n\t"
 #ifdef __x86_64__
     "pop %r8  \n\t"
     "pop %r9  \n\t"
diff --git a/kvm/user/test/x86/vmexit.c b/kvm/user/test/x86/vmexit.c
index bd57bfa..981d6c1 100644
--- a/kvm/user/test/x86/vmexit.c
+++ b/kvm/user/test/x86/vmexit.c
@@ -31,7 +31,7 @@ int main()
 
 	t1 = rdtsc();
 	for (i = 0; i < N; ++i)
-		asm volatile ("push %%"R"bx; cpuid; pop %%"R"bx"
+		asm volatile ("push %%"R "bx; cpuid; pop %%"R "bx"
 			      : : : "eax", "ecx", "edx");
 	t2 = rdtsc();
 	printf("vmexit latency: %d\n", (int)((t2 - t1) / N));
-- 
1.6.0.6

[PATCH 03/17] Fix build breakage on non-device-deassignment capable host kernels

[Anthony Liguori]

From: Han, Weidong <weidong.han@intel.com>

kvm_deassign_pci_device is defined when KVM_CAP_DEVICE_DEASSIGNMENT is defined.

Signed-off-by: Weidong Han <weidong.han@intel.com>
Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index b7cbcec..56d70b4 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -551,6 +551,7 @@ static int assign_irq(AssignedDevInfo *adev)
 
 static void deassign_device(AssignedDevInfo *adev)
 {
+#ifdef KVM_CAP_DEVICE_DEASSIGNMENT
     struct kvm_assigned_pci_dev assigned_dev_data;
     AssignedDevice *dev = adev->assigned_dev;
     int r;
@@ -563,6 +564,7 @@ static void deassign_device(AssignedDevInfo *adev)
     if (r < 0)
 	fprintf(stderr, "Failed to deassign device \"%s\" : %s\n",
                 adev->name, strerror(-r));
+#endif
 }
 
 void remove_assigned_device(AssignedDevInfo *adev)
-- 
1.6.0.6

Re: [PATCH 03/17] Fix build breakage on non-device-deassignment capable host kernels

[Avi Kivity]

Anthony Liguori wrote:
> From: Han, Weidong <weidong.han@intel.com>
>
> kvm_deassign_pci_device is defined when KVM_CAP_DEVICE_DEASSIGNMENT is defined.
>   

Unneeded, since the headers are carried in qemu-kvm.git.

-- 
error compiling committee.c: too many arguments to function

[PATCH 04/17] kvm: extboot: Update number of HDs reported by BIOS

[Anthony Liguori]

From: Gleb Natapov <gleb@redhat.com>

The Vista boot loaded expects the bios to report a correct count of the
bootable disks; and extboot disks are bootable.

This fixes Vista boot from virtio-blk issue.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/kvm/extboot/extboot.S b/kvm/extboot/extboot.S
index e3d1adf..1e60f68 100644
--- a/kvm/extboot/extboot.S
+++ b/kvm/extboot/extboot.S
@@ -32,6 +32,9 @@ _start:
 	xor %ax, %ax
 	mov %ax, %ds
 
+	/* there is one more bootable HD */
+	incb 0x0475
+
 	/* save old int 19 */
 	mov (0x19*4), %eax
 	mov %eax, %cs:old_int19
-- 
1.6.0.6

[PATCH 05/17] Stop/start cpus before/after devices

[Anthony Liguori]

From: Yaniv Kamay <yaniv@qumranet.com>

Stop cpus before devices when stopping the VM, start cpus after devices
when starting VM.  Otherwise a vcpu could access a stopped device.

Acked-by: Dor Laor <dlaor@redhat.com>
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/qemu-kvm.c b/qemu-kvm.c
index 6893cfe..fab00ac 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -285,7 +285,7 @@ static int all_threads_paused(void)
     return 1;
 }
 
-static void pause_all_threads(void)
+void qemu_kvm_pause_all_threads(void)
 {
     CPUState *penv = first_cpu;
 
@@ -305,7 +305,7 @@ static void pause_all_threads(void)
 	qemu_cond_wait(&qemu_pause_cond);
 }
 
-static void resume_all_threads(void)
+void qemu_kvm_resume_all_threads(void)
 {
     CPUState *penv = first_cpu;
 
@@ -319,14 +319,6 @@ static void resume_all_threads(void)
     }
 }
 
-static void kvm_vm_state_change_handler(void *context, int running, int reason)
-{
-    if (running)
-	resume_all_threads();
-    else
-	pause_all_threads();
-}
-
 static void update_regs_for_sipi(CPUState *env)
 {
     kvm_arch_update_regs_for_sipi(env);
@@ -371,7 +363,7 @@ static void qemu_kvm_system_reset(void)
 {
     CPUState *penv = first_cpu;
 
-    pause_all_threads();
+    qemu_kvm_pause_all_threads();
 
     qemu_system_reset();
 
@@ -380,7 +372,7 @@ static void qemu_kvm_system_reset(void)
         penv = (CPUState *)penv->next_cpu;
     }
 
-    resume_all_threads();
+    qemu_kvm_resume_all_threads();
 }
 
 static int kvm_main_loop_cpu(CPUState *env)
@@ -465,7 +457,6 @@ int kvm_init_ap(void)
 #ifdef TARGET_I386
     kvm_tpr_opt_setup();
 #endif
-    qemu_add_vm_change_state_handler(kvm_vm_state_change_handler, NULL);
 
     signal(SIG_IPI, sig_ipi_handler);
     return 0;
@@ -609,7 +600,7 @@ int kvm_main_loop(void)
 #endif
     }
 
-    pause_all_threads();
+    qemu_kvm_pause_all_threads();
     pthread_mutex_unlock(&qemu_mutex);
 
     return 0;
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 85f8668..6dd9448 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -119,6 +119,9 @@ int qemu_kvm_register_coalesced_mmio(target_phys_addr_t addr,
 int qemu_kvm_unregister_coalesced_mmio(target_phys_addr_t addr,
 				       unsigned int size);
 
+void qemu_kvm_pause_all_threads(void);
+void qemu_kvm_resume_all_threads(void);
+
 int kvm_coalesce_mmio_region(target_phys_addr_t start, ram_addr_t size);
 int kvm_uncoalesce_mmio_region(target_phys_addr_t start, ram_addr_t size);
 
diff --git a/vl.c b/vl.c
index 3cba8ed..0437159 100644
--- a/vl.c
+++ b/vl.c
@@ -3556,6 +3556,8 @@ void vm_start(void)
         cpu_enable_ticks();
         vm_running = 1;
         vm_state_notify(1, 0);
+        if (kvm_enabled())
+            qemu_kvm_resume_all_threads();
         qemu_rearm_alarm_timer(alarm_timer);
     }
 }
@@ -3565,6 +3567,8 @@ void vm_stop(int reason)
     if (vm_running) {
         cpu_disable_ticks();
         vm_running = 0;
+        if (kvm_enabled())
+            qemu_kvm_pause_all_threads();
         vm_state_notify(0, reason);
     }
 }
-- 
1.6.0.6

Re: [PATCH 05/17] Stop/start cpus before/after devices

[Avi Kivity]

Anthony Liguori wrote:
> From: Yaniv Kamay <yaniv@qumranet.com>
>
> Stop cpus before devices when stopping the VM, start cpus after devices
> when starting VM.  Otherwise a vcpu could access a stopped device.
>
>   

IIRC we decided this was unnecessary since everything is under 
qemu_mutex.  However we might want to be on the safe side here and be 
similar to master.

Marcelo?

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 05/17] Stop/start cpus before/after devices

[Marcelo Tosatti]

On Sun, May 17, 2009 at 07:59:51PM +0300, Avi Kivity wrote:
> Anthony Liguori wrote:
>> From: Yaniv Kamay <yaniv@qumranet.com>
>>
>> Stop cpus before devices when stopping the VM, start cpus after devices
>> when starting VM.  Otherwise a vcpu could access a stopped device.
>>
>>   
>
> IIRC we decided this was unnecessary since everything is under  
> qemu_mutex.  However we might want to be on the safe side here and be  
> similar to master.

Yes, it is not necessary. Since upstream is already on the safe side
(and the plan is to merge with it), IMO better leave this patch out.

Re: [PATCH 05/17] Stop/start cpus before/after devices

[Anthony Liguori]

Marcelo Tosatti wrote:
> On Sun, May 17, 2009 at 07:59:51PM +0300, Avi Kivity wrote:
>   
>> Anthony Liguori wrote:
>>     
>>> From: Yaniv Kamay <yaniv@qumranet.com>
>>>
>>> Stop cpus before devices when stopping the VM, start cpus after devices
>>> when starting VM.  Otherwise a vcpu could access a stopped device.
>>>
>>>   
>>>       
>> IIRC we decided this was unnecessary since everything is under  
>> qemu_mutex.  However we might want to be on the safe side here and be  
>> similar to master.
>>     
>
> Yes, it is not necessary. Since upstream is already on the safe side
> (and the plan is to merge with it), IMO better leave this patch out.
>   

Maybe it ought to be reverted from master then?

-- 
Regards,

Anthony Liguori

Re: [PATCH 05/17] Stop/start cpus before/after devices

[Avi Kivity]

Anthony Liguori wrote:
> Marcelo Tosatti wrote:
>> On Sun, May 17, 2009 at 07:59:51PM +0300, Avi Kivity wrote:
>>  
>>> Anthony Liguori wrote:
>>>    
>>>> From: Yaniv Kamay <yaniv@qumranet.com>
>>>>
>>>> Stop cpus before devices when stopping the VM, start cpus after 
>>>> devices
>>>> when starting VM.  Otherwise a vcpu could access a stopped device.
>>>>
>>>>         
>>> IIRC we decided this was unnecessary since everything is under  
>>> qemu_mutex.  However we might want to be on the safe side here and 
>>> be  similar to master.
>>>     
>>
>> Yes, it is not necessary. Since upstream is already on the safe side
>> (and the plan is to merge with it), IMO better leave this patch out.
>>   
>
> Maybe it ought to be reverted from master then?
>


Yes.  I'll do that.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 05/17] Stop/start cpus before/after devices

[Marcelo Tosatti]

On Mon, May 18, 2009 at 09:26:31AM -0500, Anthony Liguori wrote:
> Marcelo Tosatti wrote:
>> On Sun, May 17, 2009 at 07:59:51PM +0300, Avi Kivity wrote:
>>   
>>> Anthony Liguori wrote:
>>>     
>>>> From: Yaniv Kamay <yaniv@qumranet.com>
>>>>
>>>> Stop cpus before devices when stopping the VM, start cpus after devices
>>>> when starting VM.  Otherwise a vcpu could access a stopped device.
>>>>
>>>>         
>>> IIRC we decided this was unnecessary since everything is under   
>>> qemu_mutex.  However we might want to be on the safe side here and be 
>>>  similar to master.
>>>     
>>
>> Yes, it is not necessary. Since upstream is already on the safe side
>> (and the plan is to merge with it), IMO better leave this patch out.
>>   
>
> Maybe it ought to be reverted from master then?

Nope, its likelt to be useful in the future.

Re: [PATCH 05/17] Stop/start cpus before/after devices

[Avi Kivity]

Marcelo Tosatti wrote:
> On Mon, May 18, 2009 at 09:26:31AM -0500, Anthony Liguori wrote:
>   
>> Marcelo Tosatti wrote:
>>     
>>> On Sun, May 17, 2009 at 07:59:51PM +0300, Avi Kivity wrote:
>>>   
>>>       
>>>> Anthony Liguori wrote:
>>>>     
>>>>         
>>>>> From: Yaniv Kamay <yaniv@qumranet.com>
>>>>>
>>>>> Stop cpus before devices when stopping the VM, start cpus after devices
>>>>> when starting VM.  Otherwise a vcpu could access a stopped device.
>>>>>
>>>>>         
>>>>>           
>>>> IIRC we decided this was unnecessary since everything is under   
>>>> qemu_mutex.  However we might want to be on the safe side here and be 
>>>>  similar to master.
>>>>     
>>>>         
>>> Yes, it is not necessary. Since upstream is already on the safe side
>>> (and the plan is to merge with it), IMO better leave this patch out.
>>>   
>>>       
>> Maybe it ought to be reverted from master then?
>>     
>
> Nope, its likelt to be useful in the future.
>   

If we change the locking, we'll likely need to change this.  In any case 
I don't like start/stop; I prefer explicit locking.

-- 
error compiling committee.c: too many arguments to function

[PATCH 06/17] Use statfs to determine size of huge pages

[Anthony Liguori]

From: Joerg Roedel <joerg.roedel@amd.com>

The current method of finding out the size of huge pages does not work
reliably anymore. Current Linux supports more than one huge page size
but /proc/meminfo only show one of the supported sizes.
To find out the real page size used can be found by calling statfs. This
patch changes qemu to use statfs instead of parsing /proc/meminfo.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/sysemu.h b/sysemu.h
index 19464cf..4333495 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -99,7 +99,7 @@ extern int graphic_rotate;
 extern int no_quit;
 extern int semihosting_enabled;
 extern int old_param;
-extern int hpagesize;
+extern long hpagesize;
 extern const char *bootp_filename;
 
 #ifdef USE_KQEMU
diff --git a/vl.c b/vl.c
index 0437159..5d02e10 100644
--- a/vl.c
+++ b/vl.c
@@ -62,6 +62,7 @@
 #include <sys/ioctl.h>
 #include <sys/resource.h>
 #include <sys/socket.h>
+#include <sys/vfs.h>
 #include <netinet/in.h>
 #include <net/if.h>
 #if defined(__NetBSD__)
@@ -256,7 +257,7 @@ const char *mem_path = NULL;
 #ifdef MAP_POPULATE
 int mem_prealloc = 1;	/* force preallocation of physical target memory */
 #endif
-int hpagesize = 0;
+long hpagesize = 0;
 const char *cpu_vendor_string;
 #ifdef TARGET_ARM
 int old_param = 0;
@@ -4707,32 +4708,27 @@ void qemu_get_launch_info(int *argc, char ***argv, int *opt_daemonize, const cha
 }
 
 #ifdef USE_KVM
-static int gethugepagesize(void)
+
+#define HUGETLBFS_MAGIC       0x958458f6
+
+static long gethugepagesize(const char *path)
 {
-    int ret, fd;
-    char buf[4096];
-    const char *needle = "Hugepagesize:";
-    char *size;
-    unsigned long hugepagesize;
+    struct statfs fs;
+    int ret;
 
-    fd = open("/proc/meminfo", O_RDONLY);
-    if (fd < 0) {
-	perror("open");
-	exit(0);
-    }
+    do {
+	    ret = statfs(path, &fs);
+    } while (ret != 0 && errno == EINTR);
 
-    ret = read(fd, buf, sizeof(buf));
-    if (ret < 0) {
-	perror("read");
-	exit(0);
+    if (ret != 0) {
+	    perror("statfs");
+	    return 0;
     }
 
-    size = strstr(buf, needle);
-    if (!size)
-	return 0;
-    size += strlen(needle);
-    hugepagesize = strtol(size, NULL, 0);
-    return hugepagesize;
+    if (fs.f_type != HUGETLBFS_MAGIC)
+	    fprintf(stderr, "Warning: path not on HugeTLBFS: %s\n", path);
+
+    return fs.f_bsize;
 }
 
 static void *alloc_mem_area(size_t memory, unsigned long *len, const char *path)
@@ -4752,7 +4748,7 @@ static void *alloc_mem_area(size_t memory, unsigned long *len, const char *path)
     if (asprintf(&filename, "%s/kvm.XXXXXX", path) == -1)
 	return NULL;
 
-    hpagesize = gethugepagesize() * 1024;
+    hpagesize = gethugepagesize(path);
     if (!hpagesize)
 	return NULL;
 
-- 
1.6.0.6

Re: [PATCH 06/17] Use statfs to determine size of huge pages

[Avi Kivity]

Anthony Liguori wrote:
> From: Joerg Roedel <joerg.roedel@amd.com>
>
> The current method of finding out the size of huge pages does not work
> reliably anymore. Current Linux supports more than one huge page size
> but /proc/meminfo only show one of the supported sizes.
> To find out the real page size used can be found by calling statfs. This
> patch changes qemu to use statfs instead of parsing /proc/meminfo.
>   

Since we don't support 1GBpages in stable-0.10, this is unneeded.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 06/17] Use statfs to determine size of huge pages

[Joerg Roedel]

On Sun, May 17, 2009 at 08:00:40PM +0300, Avi Kivity wrote:
> Anthony Liguori wrote:
>> From: Joerg Roedel <joerg.roedel@amd.com>
>>
>> The current method of finding out the size of huge pages does not work
>> reliably anymore. Current Linux supports more than one huge page size
>> but /proc/meminfo only show one of the supported sizes.
>> To find out the real page size used can be found by calling statfs. This
>> patch changes qemu to use statfs instead of parsing /proc/meminfo.
>>   
>
> Since we don't support 1GBpages in stable-0.10, this is unneeded.

This patch is needed to run current KVM on a hugetlbfs backed with
1GB pages. Therefore I think this patch is needed. It is an improvement
over the /proc/meminfo parsing anyway and is not strictly related to
kvm kernel support for 1GB pages.

Joerg

-- 
           | Advanced Micro Devices GmbH
 Operating | Karl-Hammerschmidt-Str. 34, 85609 Dornach bei München
 System    | 
 Research  | Geschäftsführer: Thomas M. McCoy, Giuliano Meroni
 Center    | Sitz: Dornach, Gemeinde Aschheim, Landkreis München
           | Registergericht München, HRB Nr. 43632

Re: [PATCH 06/17] Use statfs to determine size of huge pages

[Anthony Liguori]

Joerg Roedel wrote:
> On Sun, May 17, 2009 at 08:00:40PM +0300, Avi Kivity wrote:
>   
>> Anthony Liguori wrote:
>>     
>>> From: Joerg Roedel <joerg.roedel@amd.com>
>>>
>>> The current method of finding out the size of huge pages does not work
>>> reliably anymore. Current Linux supports more than one huge page size
>>> but /proc/meminfo only show one of the supported sizes.
>>> To find out the real page size used can be found by calling statfs. This
>>> patch changes qemu to use statfs instead of parsing /proc/meminfo.
>>>   
>>>       
>> Since we don't support 1GBpages in stable-0.10, this is unneeded.
>>     
>
> This patch is needed to run current KVM on a hugetlbfs backed with
> 1GB pages. Therefore I think this patch is needed. It is an improvement
> over the /proc/meminfo parsing anyway and is not strictly related to
> kvm kernel support for 1GB pages.
>   

Is there any userspace support requirements for 1GB pages?

That is, if you had a 2.6.31 kernel and stable-0.10, would 1GB pages 
work (assuming this patch is backported)?

This patch could still be considered a feature vs. bug fix but I'm 
mostly curious.

Regards,

Anthony Liguori

> Joerg
>
>   


-- 
Regards,

Anthony Liguori

Re: [PATCH 06/17] Use statfs to determine size of huge pages

[Joerg Roedel]

On Mon, May 18, 2009 at 08:10:28AM -0500, Anthony Liguori wrote:
> Joerg Roedel wrote:
>> On Sun, May 17, 2009 at 08:00:40PM +0300, Avi Kivity wrote:
>>   
>>> Anthony Liguori wrote:
>>>     
>>>> From: Joerg Roedel <joerg.roedel@amd.com>
>>>>
>>>> The current method of finding out the size of huge pages does not work
>>>> reliably anymore. Current Linux supports more than one huge page size
>>>> but /proc/meminfo only show one of the supported sizes.
>>>> To find out the real page size used can be found by calling statfs. This
>>>> patch changes qemu to use statfs instead of parsing /proc/meminfo.
>>>>         
>>> Since we don't support 1GBpages in stable-0.10, this is unneeded.
>>>     
>>
>> This patch is needed to run current KVM on a hugetlbfs backed with
>> 1GB pages. Therefore I think this patch is needed. It is an improvement
>> over the /proc/meminfo parsing anyway and is not strictly related to
>> kvm kernel support for 1GB pages.
>>   
>
> Is there any userspace support requirements for 1GB pages?

The /proc/meminfo parsing code breaks when current KVM is run with
a -mempath on a 1GB backed hugetlbfs.

> That is, if you had a 2.6.31 kernel and stable-0.10, would 1GB pages  
> work (assuming this patch is backported)?

With this patch and kvm kernel support 1GB pages will work. Another
patch is needed to make it more easy to enable the pdpe1gb cpuid bit in
the guest.

Joerg

-- 
           | Advanced Micro Devices GmbH
 Operating | Karl-Hammerschmidt-Str. 34, 85609 Dornach bei München
 System    | 
 Research  | Geschäftsführer: Thomas M. McCoy, Giuliano Meroni
 Center    | Sitz: Dornach, Gemeinde Aschheim, Landkreis München
           | Registergericht München, HRB Nr. 43632

[PATCH 07/17] Fixup 4GB+ memslot large page alignment

[Anthony Liguori]

From: Marcelo Tosatti <mtosatti@redhat.com>

Need to align the 4GB+ memslot after we know its address, not before.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/hw/pc.c b/hw/pc.c
index 062c306..d6486a1 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -873,6 +873,7 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
 
     /* above 4giga memory allocation */
     if (above_4g_mem_size > 0) {
+        ram_addr = qemu_ram_alloc(above_4g_mem_size);
         if (hpagesize) {
             if (ram_addr & (hpagesize-1)) {
                 unsigned long aligned_addr;
@@ -881,7 +882,6 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
                 ram_addr = aligned_addr;
             }
         }
-        ram_addr = qemu_ram_alloc(above_4g_mem_size);
         cpu_register_physical_memory(0x100000000ULL,
                                      above_4g_mem_size,
                                      ram_addr);
-- 
1.6.0.6

[PATCH 08/17] Clear tap features on initialization

[Anthony Liguori]

From: Avi Kivity <avi@redhat.com>

tap features change how tap interprets data, so they must be cleared on
initialization to prevent old settings from interfering with new guest
instances.

Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/net.c b/net.c
index 08cd8c1..d8fb759 100644
--- a/net.c
+++ b/net.c
@@ -945,6 +945,7 @@ static TAPState *net_tap_fd_init(VLANState *vlan,
 #endif
 #ifdef TUNSETOFFLOAD
     s->vc->set_offload = tap_set_offload;
+    tap_set_offload(s->vc, 0, 0, 0, 0);
 #endif
     qemu_set_fd_handler2(s->fd, tap_can_send, tap_send, NULL, s);
     snprintf(s->vc->info_str, sizeof(s->vc->info_str), "fd=%d", fd);
-- 
1.6.0.6

[PATCH 09/17] Propagate errors on failed migration.

[Anthony Liguori]

From: Glauber Costa <glommer@redhat.com>

We're currently ignoring any errors if dirty logging fails.
Set error on migration file if we're unable to put dirty logging
on.

Signed-off-by: Glauber Costa <glommer@redhat.com>
Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/vl.c b/vl.c
index 5d02e10..dd2ae4c 100644
--- a/vl.c
+++ b/vl.c
@@ -3195,8 +3195,15 @@ static int ram_save_block(QEMUFile *f)
     int found = 0;
 
     while (addr < phys_ram_size) {
-        if (kvm_enabled() && current_addr == 0)
-            kvm_update_dirty_pages_log(); /* FIXME: propagate errors */
+        if (kvm_enabled() && current_addr == 0) {
+            int r;
+            r = kvm_update_dirty_pages_log();
+            if (r) {
+                fprintf(stderr, "%s: update dirty pages log failed %d\n", __FUNCTION__, r);
+                qemu_file_set_error(f);
+                return 0;
+            }
+        }
         if (cpu_physical_memory_get_dirty(current_addr, MIGRATION_DIRTY_FLAG)) {
             uint8_t ch;
 
-- 
1.6.0.6

[PATCH 10/17] Support unregistering memory regions larger than a slot

[Anthony Liguori]

From: Avi Kivity <avi@redhat.com>

Unbreaks vga text mode after switching from graphics mode.

Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/qemu-kvm.c b/qemu-kvm.c
index fab00ac..10e2caa 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -889,14 +889,15 @@ void kvm_cpu_register_physical_memory(target_phys_addr_t start_addr,
                                       unsigned long phys_offset)
 {
     int r = 0;
-    unsigned long area_flags = phys_offset & ~TARGET_PAGE_MASK;
+    unsigned long area_flags;
 #ifdef TARGET_I386
     struct mapping *p;
 #endif
 
     phys_offset &= ~IO_MEM_ROM;
+    area_flags = phys_offset & ~TARGET_PAGE_MASK;
 
-    if (area_flags == IO_MEM_UNASSIGNED) {
+    if (area_flags != IO_MEM_RAM) {
 #ifdef TARGET_I386
         if (must_use_aliases_source(start_addr)) {
             kvm_destroy_memory_alias(kvm_context, start_addr);
@@ -905,7 +906,19 @@ void kvm_cpu_register_physical_memory(target_phys_addr_t start_addr,
         if (must_use_aliases_target(start_addr))
             return;
 #endif
-        kvm_unregister_memory_area(kvm_context, start_addr, size);
+        while (size > 0) {
+            p = find_mapping(start_addr);
+            if (p) {
+                kvm_unregister_memory_area(kvm_context, p->phys, p->len);
+                drop_mapping(p->phys);
+            }
+            start_addr += TARGET_PAGE_SIZE;
+            if (size > TARGET_PAGE_SIZE) {
+                size -= TARGET_PAGE_SIZE;
+            } else {
+                size = 0;
+            }
+        }
         return;
     }
 
-- 
1.6.0.6

Re: [PATCH 10/17] Support unregistering memory regions larger than a slot

[Avi Kivity]

Anthony Liguori wrote:
> From: Avi Kivity <avi@redhat.com>
>
> Unbreaks vga text mode after switching from graphics mode.
>
>   

I think this was caused by a merge (and indeed the immediately preceding 
commit is a merge), so this is likely unneeded.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 10/17] Support unregistering memory regions larger than a slot

[Jan Kiszka]

Avi Kivity wrote:
> Anthony Liguori wrote:
>> From: Avi Kivity <avi@redhat.com>
>>
>> Unbreaks vga text mode after switching from graphics mode.
>>
>>   
> 
> I think this was caused by a merge (and indeed the immediately preceding
> commit is a merge), so this is likely unneeded.
> 

Nope, it also fixes the long-standing text mode corruption after reset
out of graphic mode.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

[PATCH 11/17] net: drop packet from tap device if all NICs are down

[Anthony Liguori]

From: Mark McLoughlin <markmc@redhat.com>

If you do e.g. "set_link virtio.0 down" and there are packets
pending on the tap interface, we currently buffer a packet
and constantly try and send it until the link is up again.

We actually just want to drop the packet if the NIC is down.
Upstream qemu already does this, we just differ because we
buffer packets from the tap interface.

[aliguori: rebased this patch on stable.  Mark, please review and Ack]

Reported-by: Yan Vugenfirer <yvugenfi@redhat.com>
Signed-off-by: Mark McLoughlin <markmc@redhat.com>
Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/net.c b/net.c
index d8fb759..ef3a965 100644
--- a/net.c
+++ b/net.c
@@ -414,8 +414,10 @@ int qemu_send_packet(VLANClientState *vc1, const uint8_t *buf, int size)
     hex_dump(stdout, buf, size);
 #endif
     for(vc = vlan->first_client; vc != NULL; vc = vc->next) {
-        if (vc != vc1 && !vc->link_down) {
-            if (!vc->fd_can_read || vc->fd_can_read(vc->opaque)) {
+        if (vc != vc1) {
+            if (vc->link_down) {
+                ret = 0;
+            } else  if (!vc->fd_can_read || vc->fd_can_read(vc->opaque)) {
                 vc->fd_read(vc->opaque, buf, size);
                 ret = 0;
             }
-- 
1.6.0.6

Re: [PATCH 11/17] net: drop packet from tap device if all NICs are down

[Mark McLoughlin]

On Sun, 2009-05-17 at 10:43 -0500, Anthony Liguori wrote:
> From: Mark McLoughlin <markmc@redhat.com>
> 
> If you do e.g. "set_link virtio.0 down" and there are packets
> pending on the tap interface, we currently buffer a packet
> and constantly try and send it until the link is up again.
> 
> We actually just want to drop the packet if the NIC is down.
> Upstream qemu already does this, we just differ because we
> buffer packets from the tap interface.
> 
> [aliguori: rebased this patch on stable.  Mark, please review and Ack]
> 
> Reported-by: Yan Vugenfirer <yvugenfi@redhat.com>
> Signed-off-by: Mark McLoughlin <markmc@redhat.com>
> Signed-off-by: Avi Kivity <avi@redhat.com>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

Looks good to me.

Cheers,
Mark.

[PATCH 11/17] net: drop packet from tap device if all NICs are down

[Anthony Liguori]

From: Mark McLoughlin <markmc@redhat.com>

If you do e.g. "set_link virtio.0 down" and there are packets
pending on the tap interface, we currently buffer a packet
and constantly try and send it until the link is up again.

We actually just want to drop the packet if the NIC is down.
Upstream qemu already does this, we just differ because we
buffer packets from the tap interface.

[aliguori: rebased this patch on stable]

Reported-by: Yan Vugenfirer <yvugenfi@redhat.com>
Signed-off-by: Mark McLoughlin <markmc@redhat.com>
Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/net.c b/net.c
index d8fb759..ef3a965 100644
--- a/net.c
+++ b/net.c
@@ -414,8 +414,10 @@ int qemu_send_packet(VLANClientState *vc1, const uint8_t *buf, int size)
     hex_dump(stdout, buf, size);
 #endif
     for(vc = vlan->first_client; vc != NULL; vc = vc->next) {
-        if (vc != vc1 && !vc->link_down) {
-            if (!vc->fd_can_read || vc->fd_can_read(vc->opaque)) {
+        if (vc != vc1) {
+            if (vc->link_down) {
+                ret = 0;
+            } else  if (!vc->fd_can_read || vc->fd_can_read(vc->opaque)) {
                 vc->fd_read(vc->opaque, buf, size);
                 ret = 0;
             }
-- 
1.6.0.6

[PATCH 12/17] Increment virtio-net savevm version to avoid conflict with upstream QEMU.

[Anthony Liguori]

When TAP_VNET_HDR eventually merges into upstream QEMU, it cannot change the
format of the version 6 savevm data.  This means that we're going to have to
bump the version up to 7.  I'm happy to reserve version 7 as having TAP_VNET_HDR
support to allow time to include this support in upstream QEMU.

For those shipping products based on KVM though, it's important that we do not
conflict with upstream QEMU versioning or else it's going to result in breakage
of backwards compatibility.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 09e4b89..27620ba 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -21,7 +21,12 @@
 
 #define TAP_VNET_HDR
 
-#define VIRTIO_NET_VM_VERSION    6
+/* Version 7 has TAP_VNET_HDR support.  This is reserved in upstream QEMU to
+ * avoid future conflict.
+ * We can't assume verisons > 7 have TAP_VNET_HDR support until this is merged
+ * in upstream QEMU.
+ */
+#define VIRTIO_NET_VM_VERSION    7
 
 #define MAC_TABLE_ENTRIES    32
 #define MAX_VLAN    (1 << 12)   /* Per 802.1Q definition */
@@ -652,8 +657,9 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
         qemu_get_buffer(f, (uint8_t *)n->vlans, MAX_VLAN >> 3);
 
 #ifdef TAP_VNET_HDR
-    if (qemu_get_be32(f))
+    if (version_id == 7 && qemu_get_be32(f)) {
         tap_using_vnet_hdr(n->vc->vlan->first_client, 1);
+    }
 #endif
 
     if (n->tx_timer_active) {
-- 
1.6.0.6

Re: [PATCH 12/17] Increment virtio-net savevm version to avoid conflict with upstream QEMU.

[Avi Kivity]

Anthony Liguori wrote:
> When TAP_VNET_HDR eventually merges into upstream QEMU, it cannot change the
> format of the version 6 savevm data.  This means that we're going to have to
> bump the version up to 7.  I'm happy to reserve version 7 as having TAP_VNET_HDR
> support to allow time to include this support in upstream QEMU.
>
> For those shipping products based on KVM though, it's important that we do not
> conflict with upstream QEMU versioning or else it's going to result in breakage
> of backwards compatibility.
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Avi Kivity <avi@redhat.com>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>
>   

Duplicate signoff.



-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 12/17] Increment virtio-net savevm version to avoid conflict with upstream QEMU.

[Anthony Liguori]

Avi Kivity wrote:
> Anthony Liguori wrote:
>> When TAP_VNET_HDR eventually merges into upstream QEMU, it cannot 
>> change the
>> format of the version 6 savevm data.  This means that we're going to 
>> have to
>> bump the version up to 7.  I'm happy to reserve version 7 as having 
>> TAP_VNET_HDR
>> support to allow time to include this support in upstream QEMU.
>>
>> For those shipping products based on KVM though, it's important that 
>> we do not
>> conflict with upstream QEMU versioning or else it's going to result 
>> in breakage
>> of backwards compatibility.
>>
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> Signed-off-by: Avi Kivity <avi@redhat.com>
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>>
>>   
>
> Duplicate signoff.

Do you care enough for me to resubmit?

Regards,

Anthony Liguori

Re: [PATCH 12/17] Increment virtio-net savevm version to avoid conflict with upstream QEMU.

[Avi Kivity]

Anthony Liguori wrote:
> Avi Kivity wrote:
>> Anthony Liguori wrote:
>>> When TAP_VNET_HDR eventually merges into upstream QEMU, it cannot 
>>> change the
>>> format of the version 6 savevm data.  This means that we're going to 
>>> have to
>>> bump the version up to 7.  I'm happy to reserve version 7 as having 
>>> TAP_VNET_HDR
>>> support to allow time to include this support in upstream QEMU.
>>>
>>> For those shipping products based on KVM though, it's important that 
>>> we do not
>>> conflict with upstream QEMU versioning or else it's going to result 
>>> in breakage
>>> of backwards compatibility.
>>>
>>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>>> Signed-off-by: Avi Kivity <avi@redhat.com>
>>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>>>
>>>   
>>
>> Duplicate signoff.
>
> Do you care enough for me to resubmit?
>

Not at all.  Just being annoying :)

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

[PATCH 13/17] Increment version id for CPU save state

[Anthony Liguori]

9 is reserved for KVM.  KVM cannot support migration from any other version.

[aliguori: incorporated Glauber's fix for save/restore breakage]

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Avi Kivity <avi@redhat.com>

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 2f8b4f6..367b4be 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -836,7 +836,9 @@ static inline int cpu_get_time_fast(void)
 #define cpu_signal_handler cpu_x86_signal_handler
 #define cpu_list x86_cpu_list
 
-#define CPU_SAVE_VERSION 8
+/* CPU_SAVE_VERSION 9 is reserved for KVM.  This is to avoid breakage as KVM
+ * merges into upstream QEMU */
+#define CPU_SAVE_VERSION 9
 
 /* MMU modes definitions */
 #define MMU_MODE0_SUFFIX _kernel
diff --git a/target-i386/machine.c b/target-i386/machine.c
index a006286..5713659 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -191,7 +191,11 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
     int32_t a20_mask;
 
     if (version_id != 3 && version_id != 4 && version_id != 5
-        && version_id != 6 && version_id != 7 && version_id != 8)
+        && version_id != 6 && version_id != 7 && version_id != 8
+        && version_id != 9)
+        return -EINVAL;
+    /* KVM cannot accept migrations from QEMU today */
+    if (version_id != 9)
         return -EINVAL;
     for(i = 0; i < CPU_NB_REGS; i++)
         qemu_get_betls(f, &env->regs[i]);
-- 
1.6.0.6

Re: [PATCH 13/17] Increment version id for CPU save state

[Avi Kivity]

Anthony Liguori wrote:
> 9 is reserved for KVM.  KVM cannot support migration from any other version.
>
> [aliguori: incorporated Glauber's fix for save/restore breakage]
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Avi Kivity <avi@redhat.com>
>
>   

I reverted this in master, so the end result is different.  Please redo 
this once it is resolved in master (in any case fixing in stable before 
master is fixed should be discouraged).

-- 
error compiling committee.c: too many arguments to function

[PATCH 14/17] Fix build when objdir != srcdir

[Anthony Liguori]

This requires adding the necessary bits to configure to create the directories
and symlinks for libkvm.  It also requires sticking KVM_CFLAGS in
config-host.mak to ensure that it gets the right set of includes for the
kernel headers.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/configure b/configure
index 04e072b..23ac0ef 100755
--- a/configure
+++ b/configure
@@ -499,7 +499,7 @@ if test "$werror" = "yes" ; then
 CFLAGS="$CFLAGS -Werror"
 fi
 
-CFLAGS="$CFLAGS -I$(readlink -f "kvm/libkvm")"
+CFLAGS="$CFLAGS -I$(readlink -f "$source_path/kvm/libkvm")"
 
 if test "$solaris" = "no" ; then
     if ld --version 2>/dev/null | grep "GNU ld" >/dev/null 2>/dev/null ; then
@@ -1580,6 +1580,11 @@ bsd)
 ;;
 esac
 
+# this is a temp hack needed for libkvm
+if test "$kvm" = "yes" ; then
+    echo "KVM_CFLAGS=$kvm_cflags" >> $config_mak
+fi
+
 tools=
 if test `expr "$target_list" : ".*softmmu.*"` != 0 ; then
   tools="qemu-img\$(EXESUF) $tools"
@@ -1945,10 +1950,11 @@ done # for target in $targets
 
 # build tree in object directory if source path is different from current one
 if test "$source_path_used" = "yes" ; then
-    DIRS="tests tests/cris slirp audio"
+    DIRS="tests tests/cris slirp audio kvm/libkvm"
     FILES="Makefile tests/Makefile"
     FILES="$FILES tests/cris/Makefile tests/cris/.gdbinit"
     FILES="$FILES tests/test-mmap.c"
+    FILES="$FILES kvm/libkvm/Makefile"
     for dir in $DIRS ; do
             mkdir -p $dir
     done
diff --git a/kvm/libkvm/Makefile b/kvm/libkvm/Makefile
index 727ce48..8de7eaf 100644
--- a/kvm/libkvm/Makefile
+++ b/kvm/libkvm/Makefile
@@ -1,5 +1,12 @@
 include ../../config-host.mak
-include config-$(ARCH).mak
+ifneq ($(VPATH),)
+srcdir=$(VPATH)/kvm/libkvm
+else
+srcdir=.
+endif
+
+include $(srcdir)/config-$(ARCH).mak
+
 
 # libkvm is not -Wredundant-decls friendly yet
 CFLAGS += -Wno-redundant-decls
@@ -18,6 +25,8 @@ LDFLAGS += $(CFLAGS)
 
 CXXFLAGS = $(autodepend-flags)
 
+VPATH:=$(VPATH)/kvm/libkvm
+
 autodepend-flags = -MMD -MF $(dir $*).$(notdir $*).d
 
 
-- 
1.6.0.6

[PATCH 15/17] Fix loading extboot option rom

[Anthony Liguori]

From: Avi Kivity <avi@redhat.com>

The buffer that is used to store the extboot filename is later overwritten
by the vga rom loading code.  Use strdup() to keep our filename.

Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/hw/pc.c b/hw/pc.c
index d6486a1..626450c 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -944,7 +944,7 @@ vga_bios_error:
 
     if (extboot_drive != -1) {
 	snprintf(buf, sizeof(buf), "%s/%s", bios_dir, EXTBOOT_FILENAME);
-        option_rom[nb_option_roms++] = buf;
+        option_rom[nb_option_roms++] = strdup(buf);
     }
 
     {
-- 
1.6.0.6

Re: [PATCH 15/17] Fix loading extboot option rom

[Avi Kivity]

Anthony Liguori wrote:
> From: Avi Kivity <avi@redhat.com>
>
> The buffer that is used to store the extboot filename is later overwritten
> by the vga rom loading code.  Use strdup() to keep our filename.
>   

This is a post-merge fixed, so likely unneeded on stable-0.10.  Would be 
good to check though.


-- 
error compiling committee.c: too many arguments to function

[PATCH 16/17] virtio-net: Make save/load format independent of build options

[Anthony Liguori]

From: Alex Williamson <alex.williamson@hp.com>

We can't rely on build switches to tell us if a save image
includes a given field.

Signed-off-by: Alex Williamson <alex.williamson@hp.com>
Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 27620ba..2da4c47 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -616,6 +616,8 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
 
 #ifdef TAP_VNET_HDR
     qemu_put_be32(f, tap_has_vnet_hdr(n->vc->vlan->first_client));
+#else
+    qemu_put_be32(f, 0);
 #endif
 }
 
@@ -660,6 +662,9 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
     if (version_id == 7 && qemu_get_be32(f)) {
         tap_using_vnet_hdr(n->vc->vlan->first_client, 1);
     }
+#else
+    /* FIXME: error out if nonzero? */
+    qemu_get_be32(f);
 #endif
 
     if (n->tx_timer_active) {
-- 
1.6.0.6

[PATCH 17/17] virtio-net: Refuse loading if the source requires vnet header and the host doesn't support it

[Anthony Liguori]

From: Alex Williamson <alex.williamson@hp.com>

Bail if the saved image requires vnet header support.

Signed-off-by: Alex Williamson <alex.williamson@hp.com>
Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 2da4c47..f65ecd7 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -658,14 +658,15 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
     if (version_id >= 6)
         qemu_get_buffer(f, (uint8_t *)n->vlans, MAX_VLAN >> 3);
 
-#ifdef TAP_VNET_HDR
     if (version_id == 7 && qemu_get_be32(f)) {
+#ifdef TAP_VNET_HDR
         tap_using_vnet_hdr(n->vc->vlan->first_client, 1);
-    }
 #else
-    /* FIXME: error out if nonzero? */
-    qemu_get_be32(f);
+        fprintf(stderr,
+                "virtio-net: saved image requires vnet header support\n");
+        exit(1);
 #endif
+    }
 
     if (n->tx_timer_active) {
         qemu_mod_timer(n->tx_timer,
-- 
1.6.0.6

Re: [PATCH 0/17][REVIEW] Backport candidates for stable-0.10

[Avi Kivity]

Anthony Liguori wrote:
> kvm-stable-start is a local tag to:
>
>   commit 8d890e234cdf5860395c4cb93b831ae37aab41e9
>   Author: Zhang, Yang <yang.zhang@intel.com>
>   Date:   Thu Mar 5 15:10:22 2009 +0800
>
>       kvm: external module: add kvm-ia64.c to hack files list
>
> As best I can tell, the is the point where KVM's stable branch forked from the
> master branch.  If anyone knows a better way to figure this out in git, please
> let me know.
>   

$ git merge-base stable-0.10 master
8d890e234cdf5860395c4cb93b831ae37aab41e9

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 0/17][REVIEW] Backport candidates for stable-0.10

[Anthony Liguori]

Avi Kivity wrote:
> Anthony Liguori wrote:
>> kvm-stable-start is a local tag to:
>>
>>   commit 8d890e234cdf5860395c4cb93b831ae37aab41e9
>>   Author: Zhang, Yang <yang.zhang@intel.com>
>>   Date:   Thu Mar 5 15:10:22 2009 +0800
>>
>>       kvm: external module: add kvm-ia64.c to hack files list
>>
>> As best I can tell, the is the point where KVM's stable branch forked 
>> from the
>> master branch.  If anyone knows a better way to figure this out in 
>> git, please
>> let me know.
>>   
>
> $ git merge-base stable-0.10 master
> 8d890e234cdf5860395c4cb93b831ae37aab41e9

Ah, that would have saved me some headache :-)

Thanks.

Regards,

Anthony Liguori

Re: [PATCH 0/17][REVIEW] Backport candidates for stable-0.10

[Avi Kivity]

Anthony Liguori wrote:
> This is a set of patches from master that I think are candidates for the
> stable-0.10 branch.   They're all KVM specific.
>
> I tried to be conservative and didn't include any ia64, PPC, or passthrough
> fixes.  I don't know the state of those subsystems well enough to judge whether
> the commits help or not.
>
> To detect set of candidates to consider, I used the following git command:
>
>  git log ^kvm-stable-start origin/master..kvm
>
>   

'git log origin/stable-0.10..origin/master' would have given you the 
same results ("all commits in master not present in stable").

I commented on some patches; all the rest seem appropriate to me.  If 
you (and other reviewers) agree, I'll apply these patches.  Thanks for 
doing this.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 0/17][REVIEW] Backport candidates for stable-0.10

[Anthony Liguori]

Avi Kivity wrote:
> Anthony Liguori wrote:
>> This is a set of patches from master that I think are candidates for the
>> stable-0.10 branch.   They're all KVM specific.
>>
>> I tried to be conservative and didn't include any ia64, PPC, or 
>> passthrough
>> fixes.  I don't know the state of those subsystems well enough to 
>> judge whether
>> the commits help or not.
>>
>> To detect set of candidates to consider, I used the following git 
>> command:
>>
>>  git log ^kvm-stable-start origin/master..kvm
>>
>>   
>
> 'git log origin/stable-0.10..origin/master' would have given you the 
> same results ("all commits in master not present in stable").

You need to exclude commits from qemu/master too.

git log ^qemu/master origin/stable-0.10..origin/master

Gives the same result as what I did.

Regards,

Anthony Liguori

> I commented on some patches; all the rest seem appropriate to me.  If 
> you (and other reviewers) agree, I'll apply these patches.  Thanks for 
> doing this.
>

Re: [PATCH 0/17][REVIEW] Backport candidates for stable-0.10

[Avi Kivity]

Anthony Liguori wrote:
>>
>> 'git log origin/stable-0.10..origin/master' would have given you the 
>> same results ("all commits in master not present in stable").
>
> You need to exclude commits from qemu/master too.
>
> git log ^qemu/master origin/stable-0.10..origin/master
>
> Gives the same result as what I did.

Yes, you're right.  There's an assumption there that upstream commits 
find there way to upstream/stable and thence to qemu-kvm stable.  An 
assumption that fortunately holds.

I like how things work out.  It would have been pretty difficult for me 
to maintain a stable branch without the qemu upstream stable branch, as 
I could only realistically backport the stuff that goes through me.

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

Re: [PATCH 0/17][REVIEW] Backport candidates for stable-0.10

[Avi Kivity]

Anthony Liguori wrote:
> This is a set of patches from master that I think are candidates for the
> stable-0.10 branch.   They're all KVM specific.
>   

I've applied all of the patches for which I had no comments previously.  
Thanks.

-- 
error compiling committee.c: too many arguments to function