[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] vbox: Fix compile errors due to the virSocketAddr series



2010/10/22 Daniel P. Berrange <berrange redhat com>:
> 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.

Thanks, pushed.

> 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
>

I think you're right. The changeset df90ca7661 addresses this bug
report https://bugzilla.redhat.com/show_bug.cgi?id=609185, but also
removed the runtime lookup for the XPCOM lib.

I'll provide a patch that improves this.

Matthias


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]