[Qemu-devel] Re: [PATCH 29/41] virtio-net: in_use and first_multi only handle unsigned values
On Wed, Dec 02, 2009 at 01:04:27PM +0100, Juan Quintela wrote:
And they were saved/loaded as unsigned already
I'm not sure how does on save/load a value as unsigned.
Could you please provide motivation for this
and similar changes?
Not that it matters very much, but int is slightly cleaner
than uint32_t for something that is only 1 or 0.
Signed-off-by: Juan Quintela quint...@redhat.com
---
hw/virtio-net.c |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 05cc67f..589ea80 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -50,8 +50,8 @@ typedef struct VirtIONet
uint8_t nouni;
uint8_t nobcast;
struct {
-int in_use;
-int first_multi;
+uint32_t in_use;
+uint32_t first_multi;
uint8_t multi_overflow;
uint8_t uni_overflow;
uint8_t *macs;
@@ -715,7 +715,7 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
qemu_put_be16s(f, n-status);
qemu_put_8s(f, n-promisc);
qemu_put_8s(f, n-allmulti);
-qemu_put_be32(f, n-mac_table.in_use);
+qemu_put_be32s(f, n-mac_table.in_use);
qemu_put_buffer(f, n-mac_table.macs, n-mac_table.in_use * ETH_ALEN);
qemu_put_buffer(f, n-vlans, MAX_VLAN 3);
qemu_put_be32(f, n-has_vnet_hdr);
@@ -756,7 +756,7 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int
version_id)
}
if (version_id = 5) {
-n-mac_table.in_use = qemu_get_be32(f);
+qemu_get_be32s(f, n-mac_table.in_use);
qemu_get_buffer(f, n-mac_table.macs,
n-mac_table.in_use * ETH_ALEN);
}
--
1.6.5.2
[Qemu-devel] Re: [PATCH 29/41] virtio-net: in_use and first_multi only handle unsigned values
Michael S. Tsirkin m...@redhat.com wrote:
On Wed, Dec 02, 2009 at 01:04:27PM +0100, Juan Quintela wrote:
And they were saved/loaded as unsigned already
I'm not sure how does on save/load a value as unsigned.
Could you please provide motivation for this
and similar changes?
Not that it matters very much, but int is slightly cleaner
than uint32_t for something that is only 1 or 0.
Signed-off-by: Juan Quintela quint...@redhat.com
---
hw/virtio-net.c |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 05cc67f..589ea80 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -50,8 +50,8 @@ typedef struct VirtIONet
uint8_t nouni;
uint8_t nobcast;
struct {
-int in_use;
-int first_multi;
+uint32_t in_use;
+uint32_t first_multi;
uint8_t multi_overflow;
uint8_t uni_overflow;
uint8_t *macs;
@@ -715,7 +715,7 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
qemu_put_be16s(f, n-status);
qemu_put_8s(f, n-promisc);
qemu_put_8s(f, n-allmulti);
-qemu_put_be32(f, n-mac_table.in_use);
This is why I hate this function.
+qemu_put_be32s(f, n-mac_table.in_use);
This is the right one.
We can change things to be int32_t if that makes more sense (they were sent
as uint32).
vmstate checks that the type of the value that you sent and the function
that you use for sending match. In this case, it was sending a int32_t
with the function to send uint32_t. In this particular case it didn't
matter (value is 0/1). But vmstate don't know what cases matter/don't
matter. It just test _always_ that you are using the right function for
your type.
If you think that it is better to change the type of the value to
int32_t and change the functions, that is also ok with me.
What I care is that type of function and field are the same.
Later, Juan.
[Qemu-devel] Re: [PATCH 29/41] virtio-net: in_use and first_multi only handle unsigned values
On Wed, Dec 02, 2009 at 07:30:18PM +0100, Juan Quintela wrote:
Michael S. Tsirkin m...@redhat.com wrote:
On Wed, Dec 02, 2009 at 01:04:27PM +0100, Juan Quintela wrote:
And they were saved/loaded as unsigned already
I'm not sure how does on save/load a value as unsigned.
Could you please provide motivation for this
and similar changes?
Not that it matters very much, but int is slightly cleaner
than uint32_t for something that is only 1 or 0.
Signed-off-by: Juan Quintela quint...@redhat.com
---
hw/virtio-net.c |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 05cc67f..589ea80 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -50,8 +50,8 @@ typedef struct VirtIONet
uint8_t nouni;
uint8_t nobcast;
struct {
-int in_use;
-int first_multi;
+uint32_t in_use;
+uint32_t first_multi;
uint8_t multi_overflow;
uint8_t uni_overflow;
uint8_t *macs;
@@ -715,7 +715,7 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
qemu_put_be16s(f, n-status);
qemu_put_8s(f, n-promisc);
qemu_put_8s(f, n-allmulti);
-qemu_put_be32(f, n-mac_table.in_use);
This is why I hate this function.
+qemu_put_be32s(f, n-mac_table.in_use);
This is the right one.
We can change things to be int32_t if that makes more sense (they were sent
as uint32).
vmstate checks that the type of the value that you sent and the function
that you use for sending match. In this case, it was sending a int32_t
with the function to send uint32_t. In this particular case it didn't
matter (value is 0/1). But vmstate don't know what cases matter/don't
matter. It just test _always_ that you are using the right function for
your type.
If you think that it is better to change the type of the value to
int32_t and change the functions, that is also ok with me.
What I care is that type of function and field are the same.
Later, Juan.
int is a better type than int32 here.
Please make the save/load functions match.
If you like, convert it to bool.
--
MST
[Qemu-devel] Re: [PATCH 29/41] virtio-net: in_use and first_multi only handle unsigned values
Michael S. Tsirkin m...@redhat.com wrote: On Wed, Dec 02, 2009 at 07:30:18PM +0100, Juan Quintela wrote: We can change things to be int32_t if that makes more sense (they were sent as uint32). vmstate checks that the type of the value that you sent and the function that you use for sending match. In this case, it was sending a int32_t with the function to send uint32_t. In this particular case it didn't matter (value is 0/1). But vmstate don't know what cases matter/don't matter. It just test _always_ that you are using the right function for your type. If you think that it is better to change the type of the value to int32_t and change the functions, that is also ok with me. What I care is that type of function and field are the same. Later, Juan. int is a better type than int32 here. will switch to int (I really hope that int32_t == int in all architectures that we are interested in) Please make the save/load functions match. If you like, convert it to bool. bool is only 1 byte, not four. I wasn't the one using 4 bytes for a bool. Later, Juan.
[Qemu-devel] Re: [PATCH 29/41] virtio-net: in_use and first_multi only handle unsigned values
On Wed, Dec 02, 2009 at 07:55:51PM +0100, Juan Quintela wrote: Michael S. Tsirkin m...@redhat.com wrote: On Wed, Dec 02, 2009 at 07:30:18PM +0100, Juan Quintela wrote: We can change things to be int32_t if that makes more sense (they were sent as uint32). vmstate checks that the type of the value that you sent and the function that you use for sending match. In this case, it was sending a int32_t with the function to send uint32_t. In this particular case it didn't matter (value is 0/1). But vmstate don't know what cases matter/don't matter. It just test _always_ that you are using the right function for your type. If you think that it is better to change the type of the value to int32_t and change the functions, that is also ok with me. What I care is that type of function and field are the same. Later, Juan. int is a better type than int32 here. will switch to int (I really hope that int32_t == int in all architectures that we are interested in) Please make the save/load functions match. If you like, convert it to bool. bool is only 1 byte, not four. I wasn't the one using 4 bytes for a bool. Later, Juan. So just range check it. The fact we leave some space in format does not mean we must waste memory at runtime. -- MST
