Patchwork [06/10] kvm: remove unused variables

login
register
mail settings
Submitter Michael S. Tsirkin
Date June 14, 2011, 5:36 p.m.
Message ID <b8c61957187f86291291abfaed88f44ac8d41b95.1308072799.git.mst@redhat.com>
Download mbox | patch
Permalink /patch/100448/
State New
Headers show

Comments

Michael S. Tsirkin - June 14, 2011, 5:36 p.m.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio-pci.h   |    8 +++++---
 target-i386/kvm.c |    3 +--
 2 files changed, 6 insertions(+), 5 deletions(-)
Kevin Wolf - June 15, 2011, 7:38 a.m.
Am 14.06.2011 19:36, schrieb Michael S. Tsirkin:
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/virtio-pci.h   |    8 +++++---
>  target-i386/kvm.c |    3 +--
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/virtio-pci.h b/hw/virtio-pci.h
> index a4b5fd3..b518917 100644
> --- a/hw/virtio-pci.h
> +++ b/hw/virtio-pci.h
> @@ -37,7 +37,9 @@ typedef struct {
>      bool ioeventfd_started;
>  } VirtIOPCIProxy;
>  
> -extern void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
> -                            uint16_t vendor, uint16_t device,
> -                            uint16_t class_code, uint8_t pif);
> +void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev);
> +
> +/* Virtio ABI version, if we increment this, we break the guest driver. */
> +#define VIRTIO_PCI_ABI_VERSION          0
> +
>  #endif

Is this hunk there intentionally?

Kevin


> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index faedc6c..58a70bc 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -970,7 +970,7 @@ static int kvm_get_xsave(CPUState *env)
>  #ifdef KVM_CAP_XSAVE
>      struct kvm_xsave* xsave;
>      int ret, i;
> -    uint16_t cwd, swd, twd, fop;
> +    uint16_t cwd, swd, twd;
>  
>      if (!kvm_has_xsave()) {
>          return kvm_get_fpu(env);
> @@ -986,7 +986,6 @@ static int kvm_get_xsave(CPUState *env)
>      cwd = (uint16_t)xsave->region[0];
>      swd = (uint16_t)(xsave->region[0] >> 16);
>      twd = (uint16_t)xsave->region[1];
> -    fop = (uint16_t)(xsave->region[1] >> 16);
>      env->fpstt = (swd >> 11) & 7;
>      env->fpus = swd;
>      env->fpuc = cwd;
Michael S. Tsirkin - June 15, 2011, 8:25 a.m.
On Wed, Jun 15, 2011 at 09:38:39AM +0200, Kevin Wolf wrote:
> Am 14.06.2011 19:36, schrieb Michael S. Tsirkin:
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  hw/virtio-pci.h   |    8 +++++---
> >  target-i386/kvm.c |    3 +--
> >  2 files changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hw/virtio-pci.h b/hw/virtio-pci.h
> > index a4b5fd3..b518917 100644
> > --- a/hw/virtio-pci.h
> > +++ b/hw/virtio-pci.h
> > @@ -37,7 +37,9 @@ typedef struct {
> >      bool ioeventfd_started;
> >  } VirtIOPCIProxy;
> >  
> > -extern void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
> > -                            uint16_t vendor, uint16_t device,
> > -                            uint16_t class_code, uint8_t pif);
> > +void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev);
> > +
> > +/* Virtio ABI version, if we increment this, we break the guest driver. */
> > +#define VIRTIO_PCI_ABI_VERSION          0
> > +
> >  #endif
> 
> Is this hunk there intentionally?
> 
> Kevin

Sorry, this belongs in another patch.
Thanks for pointing this out.
Otherwise ack?

> 
> > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > index faedc6c..58a70bc 100644
> > --- a/target-i386/kvm.c
> > +++ b/target-i386/kvm.c
> > @@ -970,7 +970,7 @@ static int kvm_get_xsave(CPUState *env)
> >  #ifdef KVM_CAP_XSAVE
> >      struct kvm_xsave* xsave;
> >      int ret, i;
> > -    uint16_t cwd, swd, twd, fop;
> > +    uint16_t cwd, swd, twd;
> >  
> >      if (!kvm_has_xsave()) {
> >          return kvm_get_fpu(env);
> > @@ -986,7 +986,6 @@ static int kvm_get_xsave(CPUState *env)
> >      cwd = (uint16_t)xsave->region[0];
> >      swd = (uint16_t)(xsave->region[0] >> 16);
> >      twd = (uint16_t)xsave->region[1];
> > -    fop = (uint16_t)(xsave->region[1] >> 16);
> >      env->fpstt = (swd >> 11) & 7;
> >      env->fpus = swd;
> >      env->fpuc = cwd;
Jan Kiszka - June 15, 2011, 8:42 a.m.
On 2011-06-15 10:25, Michael S. Tsirkin wrote:
> On Wed, Jun 15, 2011 at 09:38:39AM +0200, Kevin Wolf wrote:
>> Am 14.06.2011 19:36, schrieb Michael S. Tsirkin:
>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>> ---
>>>  hw/virtio-pci.h   |    8 +++++---
>>>  target-i386/kvm.c |    3 +--
>>>  2 files changed, 6 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/virtio-pci.h b/hw/virtio-pci.h
>>> index a4b5fd3..b518917 100644
>>> --- a/hw/virtio-pci.h
>>> +++ b/hw/virtio-pci.h
>>> @@ -37,7 +37,9 @@ typedef struct {
>>>      bool ioeventfd_started;
>>>  } VirtIOPCIProxy;
>>>  
>>> -extern void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
>>> -                            uint16_t vendor, uint16_t device,
>>> -                            uint16_t class_code, uint8_t pif);
>>> +void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev);
>>> +
>>> +/* Virtio ABI version, if we increment this, we break the guest driver. */
>>> +#define VIRTIO_PCI_ABI_VERSION          0
>>> +
>>>  #endif
>>
>> Is this hunk there intentionally?
>>
>> Kevin
> 
> Sorry, this belongs in another patch.
> Thanks for pointing this out.
> Otherwise ack?

I've still the true fix pending, but there are concerns about the
associated savevm version increase for the CPU. Not sure what will
happen the next days till -rc0, so we may also go with any of these
removal patches (you weren't the first by far).

Jan
Krumme, Chris - June 15, 2011, 12:44 p.m.
On 06/14/2011 12:36 PM, Michael S. Tsirkin wrote:
> Signed-off-by: Michael S. Tsirkin<mst@redhat.com>
> ---
>   hw/virtio-pci.h   |    8 +++++---
>   target-i386/kvm.c |    3 +--
>   2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/hw/virtio-pci.h b/hw/virtio-pci.h
> index a4b5fd3..b518917 100644
> --- a/hw/virtio-pci.h
> +++ b/hw/virtio-pci.h
> @@ -37,7 +37,9 @@ typedef struct {
>       bool ioeventfd_started;
>   } VirtIOPCIProxy;
>
> -extern void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
> -                            uint16_t vendor, uint16_t device,
> -                            uint16_t class_code, uint8_t pif);
> +void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev);

Hello MST,

The above change shows why we should be including a header, rather than 
having private prototypes (w/bitrot).

> +
> +/* Virtio ABI version, if we increment this, we break the guest driver. */
> +#define VIRTIO_PCI_ABI_VERSION          0
> +

This change seems unrelated.

Thanks

Chris

>   #endif
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index faedc6c..58a70bc 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -970,7 +970,7 @@ static int kvm_get_xsave(CPUState *env)
>   #ifdef KVM_CAP_XSAVE
>       struct kvm_xsave* xsave;
>       int ret, i;
> -    uint16_t cwd, swd, twd, fop;
> +    uint16_t cwd, swd, twd;
>
>       if (!kvm_has_xsave()) {
>           return kvm_get_fpu(env);
> @@ -986,7 +986,6 @@ static int kvm_get_xsave(CPUState *env)
>       cwd = (uint16_t)xsave->region[0];
>       swd = (uint16_t)(xsave->region[0]>>  16);
>       twd = (uint16_t)xsave->region[1];
> -    fop = (uint16_t)(xsave->region[1]>>  16);
>       env->fpstt = (swd>>  11)&  7;
>       env->fpus = swd;
>       env->fpuc = cwd;
Michael S. Tsirkin - June 15, 2011, 6:29 p.m.
On Wed, Jun 15, 2011 at 07:44:48AM -0500, Chris Krumme wrote:
> On 06/14/2011 12:36 PM, Michael S. Tsirkin wrote:
> >Signed-off-by: Michael S. Tsirkin<mst@redhat.com>
> >---
> >  hw/virtio-pci.h   |    8 +++++---
> >  target-i386/kvm.c |    3 +--
> >  2 files changed, 6 insertions(+), 5 deletions(-)
> >
> >diff --git a/hw/virtio-pci.h b/hw/virtio-pci.h
> >index a4b5fd3..b518917 100644
> >--- a/hw/virtio-pci.h
> >+++ b/hw/virtio-pci.h
> >@@ -37,7 +37,9 @@ typedef struct {
> >      bool ioeventfd_started;
> >  } VirtIOPCIProxy;
> >
> >-extern void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
> >-                            uint16_t vendor, uint16_t device,
> >-                            uint16_t class_code, uint8_t pif);
> >+void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev);
> 
> Hello MST,
> 
> The above change shows why we should be including a header, rather
> than having private prototypes (w/bitrot).

Which header?

> >+
> >+/* Virtio ABI version, if we increment this, we break the guest driver. */
> >+#define VIRTIO_PCI_ABI_VERSION          0
> >+
> 
> This change seems unrelated.
> 
> Thanks
> 
> Chris

Yes.

> >  #endif
> >diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> >index faedc6c..58a70bc 100644
> >--- a/target-i386/kvm.c
> >+++ b/target-i386/kvm.c
> >@@ -970,7 +970,7 @@ static int kvm_get_xsave(CPUState *env)
> >  #ifdef KVM_CAP_XSAVE
> >      struct kvm_xsave* xsave;
> >      int ret, i;
> >-    uint16_t cwd, swd, twd, fop;
> >+    uint16_t cwd, swd, twd;
> >
> >      if (!kvm_has_xsave()) {
> >          return kvm_get_fpu(env);
> >@@ -986,7 +986,6 @@ static int kvm_get_xsave(CPUState *env)
> >      cwd = (uint16_t)xsave->region[0];
> >      swd = (uint16_t)(xsave->region[0]>>  16);
> >      twd = (uint16_t)xsave->region[1];
> >-    fop = (uint16_t)(xsave->region[1]>>  16);
> >      env->fpstt = (swd>>  11)&  7;
> >      env->fpus = swd;
> >      env->fpuc = cwd;

Patch

diff --git a/hw/virtio-pci.h b/hw/virtio-pci.h
index a4b5fd3..b518917 100644
--- a/hw/virtio-pci.h
+++ b/hw/virtio-pci.h
@@ -37,7 +37,9 @@  typedef struct {
     bool ioeventfd_started;
 } VirtIOPCIProxy;
 
-extern void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
-                            uint16_t vendor, uint16_t device,
-                            uint16_t class_code, uint8_t pif);
+void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev);
+
+/* Virtio ABI version, if we increment this, we break the guest driver. */
+#define VIRTIO_PCI_ABI_VERSION          0
+
 #endif
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index faedc6c..58a70bc 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -970,7 +970,7 @@  static int kvm_get_xsave(CPUState *env)
 #ifdef KVM_CAP_XSAVE
     struct kvm_xsave* xsave;
     int ret, i;
-    uint16_t cwd, swd, twd, fop;
+    uint16_t cwd, swd, twd;
 
     if (!kvm_has_xsave()) {
         return kvm_get_fpu(env);
@@ -986,7 +986,6 @@  static int kvm_get_xsave(CPUState *env)
     cwd = (uint16_t)xsave->region[0];
     swd = (uint16_t)(xsave->region[0] >> 16);
     twd = (uint16_t)xsave->region[1];
-    fop = (uint16_t)(xsave->region[1] >> 16);
     env->fpstt = (swd >> 11) & 7;
     env->fpus = swd;
     env->fpuc = cwd;