[Libvir] [PATCH] to run on IA64
Hi, This patch intends to run libvirt 0.2.0 on IA64. Signed-off-by: Atsushi SAKAI <sakaia@jp.fujitsu.com> Just one question, is there any plan to support big/little endian for libvirt? (related to network) Thanks Atsushi SAKAI
On Fri, Feb 16, 2007 at 07:24:19PM +0900, Atsushi SAKAI wrote:
Hi,
This patch intends to run libvirt 0.2.0 on IA64.
Signed-off-by: Atsushi SAKAI <sakaia@jp.fujitsu.com>
This patch looks fine to me - I assume its just about silencing warnings from ever stricter GCC compiles.
Just one question, is there any plan to support big/little endian for libvirt? (related to network)
The current protocol used in QEMU backend is 'native' endianness, since it is only accessible over UNIX sockets & thus doesn't need to worry about endianness mismatch. When we add off-host remote management we'll need to ensure that all the data types are fixed sizes (eg int32_t instead of int) and that we pick a fixed byteorder. Rich Jones' work is using SunRPC as the network transport which, IIRC, mandates fixed sizes, and will always use network byte order to ensure compatabilty across archs. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
Hi, Dan Thank you for your comments. 1) This patch is for rpmbuild failure on IA64. see the attached log (it is stopped in conf.c compilation) 2) About above fixes code.(endian issue) Sorry for short discripting this issue. My point is you are casting (unsigned int*) for unsigned char. In little endian, this works fine (conf.c). But big endian case, this inputs "00" for character value. Thanks Atsushi SAKAI "Daniel P. Berrange" <berrange@redhat.com> wrote:
On Fri, Feb 16, 2007 at 07:24:19PM +0900, Atsushi SAKAI wrote:
Hi,
This patch intends to run libvirt 0.2.0 on IA64.
Signed-off-by: Atsushi SAKAI <sakaia@jp.fujitsu.com>
This patch looks fine to me - I assume its just about silencing warnings from ever stricter GCC compiles.
Just one question, is there any plan to support big/little endian for libvirt? (related to network)
The current protocol used in QEMU backend is 'native' endianness, since it is only accessible over UNIX sockets & thus doesn't need to worry about endianness mismatch. When we add off-host remote management we'll need to ensure that all the data types are fixed sizes (eg int32_t instead of int) and that we pick a fixed byteorder. Rich Jones' work is using SunRPC as the network transport which, IIRC, mandates fixed sizes, and will always use network byte order to ensure compatabilty across archs.
Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Mon, Feb 19, 2007 at 05:50:51PM +0900, Atsushi SAKAI wrote:
Hi, Dan
Thank you for your comments. 1) This patch is for rpmbuild failure on IA64. see the attached log (it is stopped in conf.c compilation)
2) About above fixes code.(endian issue) Sorry for short discripting this issue. My point is you are casting (unsigned int*) for unsigned char. In little endian, this works fine?$B!!(conf.c). But big endian case, this inputs "00" for character value.
Ahhhhhhhh. That makes much more sense to me now - explains why I never saw any problem on x86_64/i686. Will fix this in CVS.
conf.c: In function 'qemudParseInterfaceXML': conf.c:450: warning: cast increases required alignment of target type conf.c:451: warning: cast increases required alignment of target type conf.c:452: warning: cast increases required alignment of target type conf.c:453: warning: cast increases required alignment of target type conf.c:454: warning: cast increases required alignment of target type conf.c:455: warning: cast increases required alignment of target type make[2]: *** [libvirt_qemud-conf.o] Error 1 make[2]: Leaving directory `/home/sakaia/rpmbuild/BUILD/libvirt-0.2.0/qemud' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/home/sakaia/rpmbuild/BUILD/libvirt-0.2.0' make: *** [all] Error 2 error: Bad exit status from /var/tmp/rpm-tmp.48812 (%build)
Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
Hi, Dan Thank you for your reply, current IA64 is in most cases (like Linux) uses Little endian at this moment. So This is for question related to virt-manager future support. Thanks Atsushi SAKAI "Daniel P. Berrange" <berrange@redhat.com> wrote:
On Mon, Feb 19, 2007 at 05:50:51PM +0900, Atsushi SAKAI wrote:
Hi, Dan
Thank you for your comments. 1) This patch is for rpmbuild failure on IA64. see the attached log (it is stopped in conf.c compilation)
2) About above fixes code.(endian issue) Sorry for short discripting this issue. My point is you are casting (unsigned int*) for unsigned char. In little endian, this works fine?$B!!(conf.c). But big endian case, this inputs "00" for character value.
Ahhhhhhhh. That makes much more sense to me now - explains why I never saw any problem on x86_64/i686. Will fix this in CVS.
conf.c: In function 'qemudParseInterfaceXML': conf.c:450: warning: cast increases required alignment of target type conf.c:451: warning: cast increases required alignment of target type conf.c:452: warning: cast increases required alignment of target type conf.c:453: warning: cast increases required alignment of target type conf.c:454: warning: cast increases required alignment of target type conf.c:455: warning: cast increases required alignment of target type make[2]: *** [libvirt_qemud-conf.o] Error 1 make[2]: Leaving directory `/home/sakaia/rpmbuild/BUILD/libvirt-0.2.0/qemud' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/home/sakaia/rpmbuild/BUILD/libvirt-0.2.0' make: *** [all] Error 2 error: Bad exit status from /var/tmp/rpm-tmp.48812 (%build)
Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
Hi there, On Fri, 2007-02-16 at 19:24 +0900, Atsushi SAKAI wrote:
- ((struct sockaddr_in *)&ifr.ifr_addr)->sin_family = AF_INET; - ((struct sockaddr_in *)&ifr.ifr_addr)->sin_addr = inaddr; + ((struct sockaddr_in *)((void *)&ifr.ifr_addr))->sin_family = AF_INET; + ((struct sockaddr_in *)((void *)&ifr.ifr_addr))->sin_addr = inaddr;
Could you explain this change? I'm not sure I follow what the problem was or how it fixes it. Thanks, Mark.
Hi, Mark I attach the message before bridge.c fixes after conf.c fix. This message solve your problem? Thanks Atsushi SAKAI Mark McLoughlin <markmc@redhat.com> wrote:
Hi there,
On Fri, 2007-02-16 at 19:24 +0900, Atsushi SAKAI wrote:
- ((struct sockaddr_in *)&ifr.ifr_addr)->sin_family = AF_INET; - ((struct sockaddr_in *)&ifr.ifr_addr)->sin_addr = inaddr; + ((struct sockaddr_in *)((void *)&ifr.ifr_addr))->sin_family = AF_INET; + ((struct sockaddr_in *)((void *)&ifr.ifr_addr))->sin_addr = inaddr;
Could you explain this change? I'm not sure I follow what the problem was or how it fixes it.
Thanks, Mark.
Hi, Thanks for that ... On Tue, 2007-02-20 at 13:51 +0900, Atsushi SAKAI wrote:
- ((struct sockaddr_in *)&ifr.ifr_addr)->sin_family = AF_INET; - ((struct sockaddr_in *)&ifr.ifr_addr)->sin_addr = inaddr; + ((struct sockaddr_in *)((void *)&ifr.ifr_addr))->sin_family = AF_INET; + ((struct sockaddr_in *)((void *)&ifr.ifr_addr))->sin_addr = inaddr;
bridge.c: In function 'brSetInetAddr': bridge.c:344: warning: cast increases required alignment of target type bridge.c:345: warning: cast increases required alignment of target type bridge.c: In function 'brGetInetAddr': bridge.c:381: warning: cast increases required alignment of target type
Okay, my understanding of this is that: - The warning is caused by -Wcast-align - Our problem is that we're casting between pointers to struct sockaddr and struct sockaddr_in, which look like struct sockaddr { unsigned short int sa_family; }; struct sockaddr_in { unsigned short int sin_family; uint16_t sin_port; struct { uint32_t s_addr } sin_addr; }; - Because of the uint32_t, struct sockaddr_in is required to be aligned to a 4 byte boundary, whereas on ia64 struct sockaddr is only required to be aligned to a 2 byte boundary - If we look at in the context of struct ifreq, though: struct ifreq { .... union { struct sockaddr ifru_addr; .... void *ifru_data; } ifr_ifru; }; #define ifr_addr ifr_ifru.ifru_addr #define ifr_data ifr_ifru.ifru_data we see that because of the void pointer in the union, the struct sockaddr is actually guaranteed to be aligned to 8 bytes and so the warning can be ignored. - I'd prefer to avoid the void pointer cast as someone could come along and wonder whether the cast is hiding a genuine problem. - So, I think I'll go ahead and do it this way instead: - ((struct sockaddr_in *)&ifr.ifr_addr)->sin_family = AF_INET; - ((struct sockaddr_in *)&ifr.ifr_addr)->sin_addr = inaddr; + ((struct sockaddr_in *)&ifr.ifr_data)->sin_family = AF_INET; + ((struct sockaddr_in *)&ifr.ifr_data)->sin_addr = inaddr; Cheers, Mark.
Hi, Mark Thank you for your suggestion. I just compiled it sucessfuly. It should be work. I will do test it tomorrow. Thanks Atsushi SAKAI Mark McLoughlin <markmc@redhat.com> wrote:
Hi, Thanks for that ...
On Tue, 2007-02-20 at 13:51 +0900, Atsushi SAKAI wrote:
- ((struct sockaddr_in *)&ifr.ifr_addr)->sin_family = AF_INET; - ((struct sockaddr_in *)&ifr.ifr_addr)->sin_addr = inaddr; + ((struct sockaddr_in *)((void *)&ifr.ifr_addr))->sin_family = AF_INET; + ((struct sockaddr_in *)((void *)&ifr.ifr_addr))->sin_addr = inaddr;
bridge.c: In function 'brSetInetAddr': bridge.c:344: warning: cast increases required alignment of target type bridge.c:345: warning: cast increases required alignment of target type bridge.c: In function 'brGetInetAddr': bridge.c:381: warning: cast increases required alignment of target type
Okay, my understanding of this is that:
- The warning is caused by -Wcast-align
- Our problem is that we're casting between pointers to struct sockaddr and struct sockaddr_in, which look like
struct sockaddr { unsigned short int sa_family; };
struct sockaddr_in { unsigned short int sin_family; uint16_t sin_port; struct { uint32_t s_addr } sin_addr; };
- Because of the uint32_t, struct sockaddr_in is required to be aligned to a 4 byte boundary, whereas on ia64 struct sockaddr is only required to be aligned to a 2 byte boundary
- If we look at in the context of struct ifreq, though:
struct ifreq { .... union { struct sockaddr ifru_addr; .... void *ifru_data; } ifr_ifru; }; #define ifr_addr ifr_ifru.ifru_addr #define ifr_data ifr_ifru.ifru_data
we see that because of the void pointer in the union, the struct sockaddr is actually guaranteed to be aligned to 8 bytes and so the warning can be ignored.
- I'd prefer to avoid the void pointer cast as someone could come along and wonder whether the cast is hiding a genuine problem.
- So, I think I'll go ahead and do it this way instead:
- ((struct sockaddr_in *)&ifr.ifr_addr)->sin_family = AF_INET; - ((struct sockaddr_in *)&ifr.ifr_addr)->sin_addr = inaddr; + ((struct sockaddr_in *)&ifr.ifr_data)->sin_family = AF_INET; + ((struct sockaddr_in *)&ifr.ifr_data)->sin_addr = inaddr;
Cheers, Mark.
Hi, Mark It works fine. Thanks Atsushi SAKAI Atsushi SAKAI <sakaia@jp.fujitsu.com> wrote:
Hi, Mark
Thank you for your suggestion. I just compiled it sucessfuly. It should be work. I will do test it tomorrow.
Thanks Atsushi SAKAI
Mark McLoughlin <markmc@redhat.com> wrote:
Hi, Thanks for that ...
On Tue, 2007-02-20 at 13:51 +0900, Atsushi SAKAI wrote:
- ((struct sockaddr_in *)&ifr.ifr_addr)->sin_family = AF_INET; - ((struct sockaddr_in *)&ifr.ifr_addr)->sin_addr = inaddr; + ((struct sockaddr_in *)((void *)&ifr.ifr_addr))->sin_family = AF_INET; + ((struct sockaddr_in *)((void *)&ifr.ifr_addr))->sin_addr = inaddr;
bridge.c: In function 'brSetInetAddr': bridge.c:344: warning: cast increases required alignment of target type bridge.c:345: warning: cast increases required alignment of target type bridge.c: In function 'brGetInetAddr': bridge.c:381: warning: cast increases required alignment of target type
Okay, my understanding of this is that:
- The warning is caused by -Wcast-align
- Our problem is that we're casting between pointers to struct sockaddr and struct sockaddr_in, which look like
struct sockaddr { unsigned short int sa_family; };
struct sockaddr_in { unsigned short int sin_family; uint16_t sin_port; struct { uint32_t s_addr } sin_addr; };
- Because of the uint32_t, struct sockaddr_in is required to be aligned to a 4 byte boundary, whereas on ia64 struct sockaddr is only required to be aligned to a 2 byte boundary
- If we look at in the context of struct ifreq, though:
struct ifreq { .... union { struct sockaddr ifru_addr; .... void *ifru_data; } ifr_ifru; }; #define ifr_addr ifr_ifru.ifru_addr #define ifr_data ifr_ifru.ifru_data
we see that because of the void pointer in the union, the struct sockaddr is actually guaranteed to be aligned to 8 bytes and so the warning can be ignored.
- I'd prefer to avoid the void pointer cast as someone could come along and wonder whether the cast is hiding a genuine problem.
- So, I think I'll go ahead and do it this way instead:
- ((struct sockaddr_in *)&ifr.ifr_addr)->sin_family = AF_INET; - ((struct sockaddr_in *)&ifr.ifr_addr)->sin_addr = inaddr; + ((struct sockaddr_in *)&ifr.ifr_data)->sin_family = AF_INET; + ((struct sockaddr_in *)&ifr.ifr_data)->sin_addr = inaddr;
Cheers, Mark.
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Feb 20, 2007 at 09:55:28AM +0000, Mark McLoughlin wrote:
Hi, Thanks for that ...
On Tue, 2007-02-20 at 13:51 +0900, Atsushi SAKAI wrote:
- ((struct sockaddr_in *)&ifr.ifr_addr)->sin_family = AF_INET; - ((struct sockaddr_in *)&ifr.ifr_addr)->sin_addr = inaddr; + ((struct sockaddr_in *)((void *)&ifr.ifr_addr))->sin_family = AF_INET; + ((struct sockaddr_in *)((void *)&ifr.ifr_addr))->sin_addr = inaddr;
bridge.c: In function 'brSetInetAddr': bridge.c:344: warning: cast increases required alignment of target type bridge.c:345: warning: cast increases required alignment of target type bridge.c: In function 'brGetInetAddr': bridge.c:381: warning: cast increases required alignment of target type
- So, I think I'll go ahead and do it this way instead:
- ((struct sockaddr_in *)&ifr.ifr_addr)->sin_family = AF_INET; - ((struct sockaddr_in *)&ifr.ifr_addr)->sin_addr = inaddr; + ((struct sockaddr_in *)&ifr.ifr_data)->sin_family = AF_INET; + ((struct sockaddr_in *)&ifr.ifr_data)->sin_addr = inaddr;
I already committed Atsushi's original fix - so go ahead & overwrite it with this updated fix Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
participants (3)
-
Atsushi SAKAI -
Daniel P. Berrange -
Mark McLoughlin