On Fri, Oct 22, 2010 at 02:23:15PM +0200, Matthias Bolte wrote:
> ---
> src/vbox/vbox_tmpl.c | 116
> +++++++++++++++++++++++++++++++++++++++++++-------
> 1 files changed, 100 insertions(+), 16 deletions(-)
>
> diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
> index 5a859a4..ddbca97 100644
> --- a/src/vbox/vbox_tmpl.c
> +++ b/src/vbox/vbox_tmpl.c
> @@ -626,6 +626,45 @@ static PRUnichar *PRUnicharFromInt(int n) {
>
> #endif /* !(VBOX_API_VERSION == 2002) */
>
> +static PRUnichar *
> +vboxSocketFormatAddrUtf16(vboxGlobalData *data, virSocketAddrPtr addr)
> +{
> + char *utf8 = NULL;
> + PRUnichar *utf16 = NULL;
> +
> + utf8 = virSocketFormatAddr(addr);
> +
> + if (utf8 == NULL) {
> + return NULL;
> + }
> +
> + VBOX_UTF8_TO_UTF16(utf8, &utf16);
> + VIR_FREE(utf8);
> +
> + return utf16;
> +}
> +
> +static int
> +vboxSocketParseAddrUtf16(vboxGlobalData *data, const PRUnichar *utf16,
> + virSocketAddrPtr addr)
> +{
> + int result = -1;
> + char *utf8 = NULL;
> +
> + VBOX_UTF16_TO_UTF8(utf16, &utf8);
> +
> + if (virSocketParseAddr(utf8, addr, AF_UNSPEC) < 0) {
> + goto cleanup;
> + }
> +
> + result = 0;
> +
> +cleanup:
> + VBOX_UTF8_FREE(utf8);
> +
> + return result;
> +}
> +
> static virCapsPtr vboxCapsInit(void) {
> struct utsname utsname;
> virCapsPtr caps;
> @@ -7073,8 +7112,8 @@ static virNetworkPtr
> vboxNetworkDefineCreateXML(virConnectPtr conn, const char *
> * with contigious address space from start to end
> */
> if ((def->nranges >= 1) &&
> - (def->ranges[0].start) &&
> - (def->ranges[0].end)) {
> + VIR_SOCKET_HAS_ADDR(&def->ranges[0].start) &&
> + VIR_SOCKET_HAS_ADDR(&def->ranges[0].end)) {
> IDHCPServer *dhcpServer = NULL;
>
> data->vboxObj->vtbl->FindDHCPServerByNetworkName(data->vboxObj,
> @@ -7094,11 +7133,21 @@ static virNetworkPtr
> vboxNetworkDefineCreateXML(virConnectPtr conn, const char *
> PRUnichar *toIPAddressUtf16 = NULL;
> PRUnichar *trunkTypeUtf16 = NULL;
>
> + ipAddressUtf16 = vboxSocketFormatAddrUtf16(data,
> &def->ipAddress);
> + networkMaskUtf16 = vboxSocketFormatAddrUtf16(data,
> &def->netmask);
> + fromIPAddressUtf16 = vboxSocketFormatAddrUtf16(data,
> &def->ranges[0].start);
> + toIPAddressUtf16 = vboxSocketFormatAddrUtf16(data,
> &def->ranges[0].end);
> +
> + if (ipAddressUtf16 == NULL || networkMaskUtf16 == NULL ||
> + fromIPAddressUtf16 == NULL || toIPAddressUtf16 == NULL) {
> + VBOX_UTF16_FREE(ipAddressUtf16);
> + VBOX_UTF16_FREE(networkMaskUtf16);
> + VBOX_UTF16_FREE(fromIPAddressUtf16);
> + VBOX_UTF16_FREE(toIPAddressUtf16);
> + VBOX_RELEASE(dhcpServer);
> + goto cleanup;
> + }
>
> - VBOX_UTF8_TO_UTF16(def->ipAddress, &ipAddressUtf16);
> - VBOX_UTF8_TO_UTF16(def->netmask, &networkMaskUtf16);
> - VBOX_UTF8_TO_UTF16(def->ranges[0].start,
> &fromIPAddressUtf16);
> - VBOX_UTF8_TO_UTF16(def->ranges[0].end, &toIPAddressUtf16);
> VBOX_UTF8_TO_UTF16("netflt", &trunkTypeUtf16);
>
> dhcpServer->vtbl->SetEnabled(dhcpServer, PR_TRUE);
> @@ -7125,12 +7174,18 @@ static virNetworkPtr
> vboxNetworkDefineCreateXML(virConnectPtr conn, const char *
> }
>
> if ((def->nhosts >= 1) &&
> - (def->hosts[0].ip)) {
> + VIR_SOCKET_HAS_ADDR(&def->hosts[0].ip)) {
> PRUnichar *ipAddressUtf16 = NULL;
> PRUnichar *networkMaskUtf16 = NULL;
>
> - VBOX_UTF8_TO_UTF16(def->netmask, &networkMaskUtf16);
> - VBOX_UTF8_TO_UTF16(def->hosts[0].ip, &ipAddressUtf16);
> + ipAddressUtf16 = vboxSocketFormatAddrUtf16(data,
> &def->hosts[0].ip);
> + networkMaskUtf16 = vboxSocketFormatAddrUtf16(data,
> &def->netmask);
> +
> + if (ipAddressUtf16 == NULL || networkMaskUtf16 == NULL) {
> + VBOX_UTF16_FREE(ipAddressUtf16);
> + VBOX_UTF16_FREE(networkMaskUtf16);
> + goto cleanup;
> + }
>
> /* Current drawback is that since EnableStaticIpConfig() sets
> * IP and enables the interface so even if the dhcpserver is not
> @@ -7393,6 +7448,7 @@ static char *vboxNetworkDumpXML(virNetworkPtr network,
> int flags ATTRIBUTE_UNUSE
> PRUnichar *networkMaskUtf16 = NULL;
> PRUnichar *fromIPAddressUtf16 = NULL;
> PRUnichar *toIPAddressUtf16 = NULL;
> + bool errorOccurred = false;
>
> dhcpServer->vtbl->GetIPAddress(dhcpServer,
> &ipAddressUtf16);
> dhcpServer->vtbl->GetNetworkMask(dhcpServer,
> &networkMaskUtf16);
> @@ -7401,15 +7457,25 @@ static char *vboxNetworkDumpXML(virNetworkPtr
> network, int flags ATTRIBUTE_UNUSE
> /* Currently virtualbox supports only one dhcp
> server per network
> * with contigious address space from start to end
> */
> - VBOX_UTF16_TO_UTF8(ipAddressUtf16, &def->ipAddress);
> - VBOX_UTF16_TO_UTF8(networkMaskUtf16, &def->netmask);
> - VBOX_UTF16_TO_UTF8(fromIPAddressUtf16,
> &def->ranges[0].start);
> - VBOX_UTF16_TO_UTF8(toIPAddressUtf16,
> &def->ranges[0].end);
> + if (vboxSocketParseAddrUtf16(data, ipAddressUtf16,
> + &def->ipAddress) < 0 ||
> + vboxSocketParseAddrUtf16(data, networkMaskUtf16,
> + &def->netmask) < 0 ||
> + vboxSocketParseAddrUtf16(data,
> fromIPAddressUtf16,
> + &def->ranges[0].start)
> < 0 ||
> + vboxSocketParseAddrUtf16(data, toIPAddressUtf16,
> + &def->ranges[0].end) <
> 0) {
> + errorOccurred = true;
> + }
>
> VBOX_UTF16_FREE(ipAddressUtf16);
> VBOX_UTF16_FREE(networkMaskUtf16);
> VBOX_UTF16_FREE(fromIPAddressUtf16);
> VBOX_UTF16_FREE(toIPAddressUtf16);
> +
> + if (errorOccurred) {
> + goto cleanup;
> + }
> } else {
> def->nranges = 0;
> virReportOOMError();
> @@ -7425,15 +7491,24 @@ static char *vboxNetworkDumpXML(virNetworkPtr
> network, int flags ATTRIBUTE_UNUSE
> } else {
> PRUnichar *macAddressUtf16 = NULL;
> PRUnichar *ipAddressUtf16 = NULL;
> + bool errorOccurred = false;
>
>
> networkInterface->vtbl->GetHardwareAddress(networkInterface,
> &macAddressUtf16);
>
> networkInterface->vtbl->GetIPAddress(networkInterface, &ipAddressUtf16);
>
> VBOX_UTF16_TO_UTF8(macAddressUtf16,
> &def->hosts[0].mac);
> - VBOX_UTF16_TO_UTF8(ipAddressUtf16,
> &def->hosts[0].ip);
> +
> + if (vboxSocketParseAddrUtf16(data,
> ipAddressUtf16,
> + &def->hosts[0].ip)
> < 0) {
> + errorOccurred = true;
> + }
>
> VBOX_UTF16_FREE(macAddressUtf16);
> VBOX_UTF16_FREE(ipAddressUtf16);
> +
> + if (errorOccurred) {
> + goto cleanup;
> + }
> }
> } else {
> def->nhosts = 0;
> @@ -7443,15 +7518,24 @@ static char *vboxNetworkDumpXML(virNetworkPtr
> network, int flags ATTRIBUTE_UNUSE
> } else {
> PRUnichar *networkMaskUtf16 = NULL;
> PRUnichar *ipAddressUtf16 = NULL;
> + bool errorOccurred = false;
>
> networkInterface->vtbl->GetNetworkMask(networkInterface,
> &networkMaskUtf16);
> networkInterface->vtbl->GetIPAddress(networkInterface,
> &ipAddressUtf16);
>
> - VBOX_UTF16_TO_UTF8(networkMaskUtf16, &def->netmask);
> - VBOX_UTF16_TO_UTF8(ipAddressUtf16, &def->ipAddress);
> + if (vboxSocketParseAddrUtf16(data, networkMaskUtf16,
> + &def->netmask) < 0 ||
> + vboxSocketParseAddrUtf16(data, ipAddressUtf16,
> + &def->ipAddress) < 0) {
> + errorOccurred = true;
> + }
>
> VBOX_UTF16_FREE(networkMaskUtf16);
> VBOX_UTF16_FREE(ipAddressUtf16);
> +
> + if (errorOccurred) {
> + goto cleanup;
> + }
> }
>
> DEBUGIID("Network UUID", vboxnet0IID);
ACK.
This highlights a problem caused by a changeset
df90ca7661b0a789bd790ccf8258a4527c13eb8d
Previously the vbox driver would be compiled by default, and any check
for the XPCOM was at runtime. Now the configure.ac script disables
vbox at build time, if it can't find the XPCOM. This is not good since
it means that if the main OS distro repositories don't contain VBox
libvirt won't get support compiled in. This then prevents a user
obtaining vbox from a 3rd party & using libvirt with it.
IMHO we should revert this changeset so we do the check at runtime
again, and simply have a configure flag that lets the user specify
extra paths to be checked at runtime.
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 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
