Re: [libvirt] [PATCH -v2 0/1] esx: Fix get/set vcpus
On Wed, Aug 15, 2018 at 06:35:47AM -0300, Marcos Paulo de Souza wrote: > Hi guys, > > this is the second version of the patch, the first one can be found here[1]. > This version addresses the comments from Matthias Bolte, making the change > simpler and cleaner. > > Let me know if there are other details that needs to change. > > [1]: https://www.redhat.com/archives/libvir-list/2018-August/msg00532.html > > Marcos Paulo de Souza (1): > esx: Make esxDomainGetVcpusFlags check flags > > src/esx/esx_driver.c | 52 > 1 file changed, 28 insertions(+), 24 deletions(-) > > -- > 2.17.1 > Checked now that libvirt is entering in "freezer", so you guys please take a look at this patch? -- Thanks, Marcos -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH -v2 1/1] esx: Make esxDomainGetVcpusFlags check flags
Before this patch, esxDomainGetVcpusFlags was not checking the flags
argument. Current, get and set vcpus was failing in ESX since it was
checking for "maxSupportedVcpus", and this configuration can be ommited
by ESXi[1]. Now, if VIR_DOMAIN_VCPU_MAXIMUM is specified in flags argument
esxDomainGetVcpusFlags the maximum number of vcpus allowed for that VM.
Otherwise, the current number of vcpus is returned.
With this patch calls to virDomainSetVcpus, virDomainGetMaxVcpus and
virDomainGetVcpusFlags to return successfull again.
[1]:https://pubs.vmware.com/vi-sdk/visdk250/ReferenceGuide/vim.host.Capability.html
Signed-off-by: Marcos Paulo de Souza
---
Changes from v1:
* Now we only have one patch instead of two
* Change esxDomainGetVcpusFlags in order to check the flags argument, instead
* of changing esxDomainGetMaxVcpus.
src/esx/esx_driver.c | 52
1 file changed, 28 insertions(+), 24 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index c2154799fa..a1ba889326 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -2547,45 +2547,49 @@ esxDomainGetVcpusFlags(virDomainPtr domain, unsigned
int flags)
{
esxPrivate *priv = domain->conn->privateData;
esxVI_String *propertyNameList = NULL;
-esxVI_ObjectContent *hostSystem = NULL;
-esxVI_DynamicProperty *dynamicProperty = NULL;
+esxVI_ObjectContent *obj = NULL;
+esxVI_Int *vcpus = NULL;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_VCPU_MAXIMUM, -1);
-if (priv->maxVcpus > 0)
-return priv->maxVcpus;
-
priv->maxVcpus = -1;
if (esxVI_EnsureSession(priv->primary) < 0)
return -1;
-if (esxVI_String_AppendValueToList(,
- "capability.maxSupportedVcpus") < 0 ||
-esxVI_LookupHostSystemProperties(priv->primary, propertyNameList,
- ) < 0) {
-goto cleanup;
-}
-for (dynamicProperty = hostSystem->propSet; dynamicProperty;
- dynamicProperty = dynamicProperty->_next) {
-if (STREQ(dynamicProperty->name, "capability.maxSupportedVcpus")) {
-if (esxVI_AnyType_ExpectType(dynamicProperty->val,
- esxVI_Type_Int) < 0) {
-goto cleanup;
-}
+if (flags & VIR_DOMAIN_VCPU_MAXIMUM) {
+if (esxVI_String_AppendValueToList(,
+ "capability.maxHostSupportedVcpus\0"
+ "capability.maxSupportedVcpus") < 0
||
+esxVI_LookupHostSystemProperties(priv->primary,
+ propertyNameList, ) < 0 ||
+esxVI_GetInt(obj, "capability.maxSupportedVcpus", ,
+ esxVI_Occurrence_OptionalItem) < 0)
+goto cleanup;
-priv->maxVcpus = dynamicProperty->val->int32;
-break;
-} else {
-VIR_WARN("Unexpected '%s' property", dynamicProperty->name);
-}
+if (!vcpus && esxVI_GetInt(obj, "capability.maxHostSupportedVcpus",
+ , esxVI_Occurrence_RequiredItem) < 0)
+goto cleanup;
+
+} else {
+if (esxVI_String_AppendValueToList(,
+ "config.hardware.numCPU\0") < 0 ||
+esxVI_LookupVirtualMachineByUuid(priv->primary, domain->uuid,
+ propertyNameList, ,
+ esxVI_Occurrence_RequiredItem) <
0 ||
+esxVI_GetInt(obj, "config.hardware.numCPU", ,
+ esxVI_Occurrence_RequiredItem) < 0)
+goto cleanup;
}
+priv->maxVcpus = vcpus->value;
+
cleanup:
esxVI_String_Free();
-esxVI_ObjectContent_Free();
+esxVI_ObjectContent_Free();
+esxVI_Int_Free();
return priv->maxVcpus;
}
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH -v2 0/1] esx: Fix get/set vcpus
Hi guys, this is the second version of the patch, the first one can be found here[1]. This version addresses the comments from Matthias Bolte, making the change simpler and cleaner. Let me know if there are other details that needs to change. [1]: https://www.redhat.com/archives/libvir-list/2018-August/msg00532.html Marcos Paulo de Souza (1): esx: Make esxDomainGetVcpusFlags check flags src/esx/esx_driver.c | 52 1 file changed, 28 insertions(+), 24 deletions(-) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] esx:Fix esxDomainGetMaxVcpus to return correct vcpus
On Mon, Aug 13, 2018 at 11:21:05PM +0200, Matthias Bolte wrote:
> 2018-08-10 5:56 GMT+02:00 Marcos Paulo de Souza :
> > Before this change, esxDomainGetMaxVcpus returned -1, which in turn
> > fails in libvirt. This commit reimplements esxDomainGetMaxVcpus instead
> > of calling esxDomainGetVcpusFlags. The implementation checks for
> > capability.maxSupportedVcpus, but as this one can be ommited in ESXi, we
> > also check for capability.maxHostSupportedVcpus. With this change,
> > virDomainSetVcpus, virDomainGetMaxVcpus and virDomainGetVcpusFlags and
> > returning correct values.
> >
> > Signed-off-by: Marcos Paulo de Souza
> > ---
> > src/esx/esx_driver.c | 36 ++--
> > 1 file changed, 34 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
> > index d5e8a7b4eb..3169314fa4 100644
> > --- a/src/esx/esx_driver.c
> > +++ b/src/esx/esx_driver.c
> > @@ -2581,8 +2581,40 @@ esxDomainGetVcpusFlags(virDomainPtr domain, unsigned
> > int flags)
> > static int
> > esxDomainGetMaxVcpus(virDomainPtr domain)
> > {
> > -return esxDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE |
> > - VIR_DOMAIN_VCPU_MAXIMUM));
> > +esxPrivate *priv = domain->conn->privateData;
> > +esxVI_String *propertyNameList = NULL;
> > +esxVI_ObjectContent *hostSystem = NULL;
> > +esxVI_Int *supportedVcpus = NULL;
> > +esxVI_Int *hostVcpus = NULL;
> > +
> > +if (esxVI_EnsureSession(priv->primary) < 0)
> > +return -1;
> > +
> > +priv->maxVcpus = -1;
> > +
> > +if (esxVI_String_AppendValueToList(,
> > +
> > "capability.maxHostSupportedVcpus\0"
> > + "capability.maxSupportedVcpus"
> > + ) < 0 ||
> > +esxVI_LookupHostSystemProperties(priv->primary, propertyNameList,
> > + ) < 0 ||
> > +esxVI_GetInt(hostSystem, "capability.maxHostSupportedVcpus",
> > + , esxVI_Occurrence_RequiredItem) < 0 ||
> > +esxVI_GetInt(hostSystem, "capability.maxSupportedVcpus",
> > + , esxVI_Occurrence_OptionalItem) < 0)
> > +
> > +goto cleanup;
> > +
> > +/* as maxSupportedVcpus is optional, check also for
> > maxHostSupportedVcpus */
> > +priv->maxVcpus = supportedVcpus ? supportedVcpus->value :
> > hostVcpus->value;
> > +
> > + cleanup:
> > +esxVI_String_Free();
> > +esxVI_ObjectContent_Free();
> > +esxVI_Int_Free();
> > +esxVI_Int_Free();
> > +
> > +return priv->maxVcpus;
> > }
>
> This is the wrong way to fix the situation. The correct way ist to
> make esxDomainGetVcpusFlags handle the VIR_DOMAIN_VCPU_MAXIMUM flag
> properly.
Thanks for the suggestions, I will send a v2 soon.
>
> --
> Matthias Bolte
> http://photron.blogspot.com
--
Thanks,
Marcos
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/11] More virAuthGet* cleanups
On Tue, Aug 14, 2018 at 01:07:45PM -0400, John Ferlan wrote:
> In a case of review collision, Michal pushed Marcos Paulo de Souza's
> series after I had reviewed, but before seeing my review (oh well):
>
> https://www.redhat.com/archives/libvir-list/2018-August/msg00874.html
>
> Rather than worry about it, here's a series of changes that I
> described in my review:
>
> https://www.redhat.com/archives/libvir-list/2018-August/msg00860.html
>
> based on top of Marcos' now pushed weries.
>
> The only thing I didn't cover in this was my comment regarding
> xenapiUtil_RequestPassword - I just left it as is.
>
> John Ferlan (11):
> util: Alter virAuthGet*Path API to check valid parameters
> util: Alter virAuthGet*Path API to check valid callback
> util: Remove invalid parameter checks from
> virAuthGet{Username|Password}
> util: Alter virAuthGet*Path API return processing
> util: Alter virAuthGet*Path API to generate auth->cb error
> esx: Don't overwrite virAuthGet{Username|Password} errors
> hyperv: Don't overwrite virAuthGet{Username|Password} errors
> phyp: Don't overwrite virAuthGet{Username|Password} errors
> xenapi: Don't overwrite virAuthGet{Username|Password} errors
> test: Don't overwrite virAuthGet{Username|Password} errors
> rpc: Don't overwrite virAuthGet{Username|Password}Path errors
>
> src/esx/esx_driver.c | 27
> src/hyperv/hyperv_driver.c| 16 +++--
> src/phyp/phyp_driver.c| 16 +++--
> src/rpc/virnetlibsshsession.c | 2 --
> src/rpc/virnetsshsession.c| 5 +--
> src/test/test_driver.c| 15 +++--
> src/util/virauth.c| 61 ---
> src/xenapi/xenapi_driver.c | 16 +++--
> 8 files changed, 67 insertions(+), 91 deletions(-)
Nice cleanup.
Reviewed-by: Marcos Paulo de Souza
Thanks,
>
> --
> 2.17.1
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
Thanks,
Marcos
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/5] virauth.c: Check for valid auth callback
On Tue, Aug 14, 2018 at 10:53:50AM -0400, John Ferlan wrote:
>
>
> On 08/02/2018 08:27 PM, Marcos Paulo de Souza wrote:
> > Instead of adding the same check for every drivers, execute the checks
> > in virAuthGetUsername and virAuthGetPassword. These funtions are called
> > when user is not set in the URI.
> >
> > Signed-off-by: Marcos Paulo de Souza
> > ---
> > src/util/virauth.c | 12
> > 1 file changed, 12 insertions(+)
> >
>
> I believe the virAuthGet{Username|Password}Path API's should be adjusted
> instead.
>
> Although not possible today as shown by the subsequent patches, if @auth
> or !auth->cb were NULL calling virAuthGetCredential and returning
> something is still possible since neither auth nor auth->cb is
> referenced. They are only referenced when it's required to ask via some
> mechanism. It's at those points we should generate the errors.
>
> Furthermore, the callers that generate their own errors based on where
> in the code they fail; however, those errors would hide memory
> allocation failures from say virAsprintf or VIR_STRDUP calls from other
> virAuth* APIs that are called.
>
> For example, calling virAuthGetConfigFilePathURI and failing to allocate
> memory would generate a memory allocation failure; however, the caller
> could overwrite that with a "{Username|Password} request failed" error
> message.
>
> Interestingly, the virAuthGetUsernamePath caller expects the error to be
> generated and doesn't generate it's own, but virAuthGetPasswordPath
> callers will generate (and overwrite) their own error.
>
> So, I think all of the error generation can be done in the *Path API's
> and a lot more cleanup is possible...
>
> First, just before the memset() (in both UsernamePath and
> PasswordPath) add the logic that would check and error for "if (!auth)
> {" with an error message such as "Missing authentication credentials"
> using VIR_ERR_INVALID_ARG for virReportError and return NULL.
>
> Next, checking and erroring for !auth->cb would only be necessary if we
> ever get past the credtype "continue;" condition. In that case the error
> message would be "Missing authentication callback" using
> VIR_ERR_INVALID_ARG for virReportError and return NULL.
>
> With those errors in place, there would be two more reasons that the
> caller would need to generate an error... First if there was no expected
> credtype for the API or if the (*cred->cb) returns < 0.
>
> Since the for loop breaks once the auth->cb is called, if we change the
> "break;" to a return cred.result and then instead of the bare return
> cred.result at the bottom we turn that into an error "Missing XXX
> credential type" (where XXX would be replaced by "VIR_CRED_AUTHNAME" and
> "VIR_CRED_PASSPHRASE or VIR_CRED_NOECHOPROMPT" using VIR_ERR_AUTH_FAILED
> for virReportError followed by return NULL.
>
> With that in place, add error processing to the auth->cb, either:
>
>virReportError(VIR_ERR_AUTH_FAILED, "%s",
>_("Username request failed"));
>
> or
>
> virReportError(VIR_ERR_AUTH_FAILED, "%s",
>_("Password request failed"));
>
> when (*auth->cb) < 0
>
> Once the above is all in place, then the callers could be adjusted to
> not generate any errors for !auth, !auth->cb, and NULL return from the
> function. Take the opportunity to clean up the callers a bit to alter
> long lines and the calls to be just:
>
> if (!( = virAuthGet*(...)))
> goto ;
>
> where the virAuthGet* call could be across multiple lines if it goes
> beyond the 80 cols, but the open/close parens {} aren't needed.
>
> The commit messages for the subsequent patches would need to be adjusted
> to note that you're removing the need to generate error messages for the
> virAuthGet{Username|Password}[Path] callers since the API's handle that.
> The test_driver and rpc/virnet*session.c callers do have different
> messages on NULL failures now, but (famous last words) that shouldn't be
> a problem.
>
> NB: The rpc/virnet[lib]sshsession.c consumers already do the !auth ||
> !auth->cb checks in the form of "if (!sess->cred || !sess->cred->cb) {".
> I would just keep those as is.
>
> Finally, not sure how it's called, but xenapiUtil_RequestPassword would
> perhaps need similar adjustments. If it's caller still overwrites the
> message, then at least the message is logged in a domain log file somewhere.
>
> Hopefully this makes sense. This on
Re: [libvirt] [PATCH] esx: Fix build when libcurl debug is enabled
On Mon, Aug 13, 2018 at 03:51:51PM +0200, Michal Prívozník wrote:
> On 08/11/2018 04:39 PM, Marcos Paulo de Souza wrote:
> > When building libvirt with libcurl debug enabled (with
> > ESX_VI__CURL__ENABLE_DEBUG_OUTPUT set), the message bellow pops up:
> >
> > make[3]: Entering directory '/mnt/data/gitroot/libvirt/src'
> > CC esx/libvirt_driver_esx_la-esx_vi.lo
> > esx/esx_vi.c: In function 'esxVI_CURL_Debug':
> > esx/esx_vi.c:191:5: error: enumeration value 'CURLINFO_SSL_DATA_IN' not
> > handled in switch [-Werror=switch-enum]
> > switch (type) {
> > ^~
> > esx/esx_vi.c:191:5: error: enumeration value 'CURLINFO_SSL_DATA_OUT' not
> > handled in switch [-Werror=switch-enum]
> > esx/esx_vi.c:191:5: error: enumeration value 'CURLINFO_END' not handled in
> > switch [-Werror=switch-enum]
> >
> > Our build requires at least libcurl 7.18.0, which is pretty stable since
> > it was release in 2008. Fix this problem by handling the mentioned enums
> > in the code.
> >
> > Signed-off-by: Marcos Paulo de Souza
> > ---
> > src/esx/esx_vi.c | 8 +++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
> > index a816c3a4f9..588670e137 100644
> > --- a/src/esx/esx_vi.c
> > +++ b/src/esx/esx_vi.c
> > @@ -163,7 +163,7 @@ esxVI_CURL_WriteBuffer(char *data, size_t size, size_t
> > nmemb, void *userdata)
> > return 0;
> > }
> >
> > -#define ESX_VI__CURL__ENABLE_DEBUG_OUTPUT 0
> > +#define ESX_VI__CURL__ENABLE_DEBUG_OUTPUT 1
>
> The part below is fine. However, I'm not sure about this one ^^. This
> turns curl debugging on by default. I'm not sure that is what we want.
You are right, this part was included by mistake. Would you want me to send a
new version with this part removed?
>
> >
> > #if ESX_VI__CURL__ENABLE_DEBUG_OUTPUT
> > static int
> > @@ -205,13 +205,19 @@ esxVI_CURL_Debug(CURL *curl ATTRIBUTE_UNUSED,
> > curl_infotype type,
> > break;
> >
> >case CURLINFO_DATA_IN:
> > + case CURLINFO_SSL_DATA_IN:
> > VIR_DEBUG("CURLINFO_DATA_IN %s", buffer);
> > break;
> >
> >case CURLINFO_DATA_OUT:
> > + case CURLINFO_SSL_DATA_OUT:
> > VIR_DEBUG("CURLINFO_DATA_OUT %s", buffer);
> > break;
> >
> > + case CURLINFO_END:
> > +VIR_DEBUG("CURLINFO_END %s", buffer);
> > +break;
> > +
> >default:
> > VIR_DEBUG("unknown");
> > break;
> >
>
>
> Michal
--
Thanks,
Marcos
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] esx: Fix nodeGetInfo so cpu model fits inside nodeinfo->model
Commit 6c0d0210cbcd5d647f0d882c07f077d444bc707d changed the behavior of
virStr*cpy* functions, so now the nodeGetInfo call fails. Version 4.1.0
(default for Fedora 28) works:
Model: Intel Core i7-4500U CPU @ 1.80G
Current master tries to write "Intel Core i7-4500U CPU @ 1.80GHz", but
the string is bigger than nodeinfo->model (which is a char[32]). So this
patch "cuts" the string, and presents the same output from 4.1.0.
Signed-off-by: Marcos Paulo de Souza
---
When testing both qemu and lxc drivers, both return "x86_64", as both call
virCapabilitiesGetNodeInfo in order the get all info the "node", but they are a
Linux machine.
When it comes to ESX, it returns a string from the ESX server, which is same
info that I can find in /proc/cpuinfo of my machine (but without the () parts):
model name : Intel(R) Core(TM) i7-4500U CPU @ 1.80GHz
The question is: should libvirt increase the model size, or just find a better
way for ESX to present this info?
src/esx/esx_driver.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index c2154799fa..03a84d7630 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -1317,6 +1317,8 @@ esxNodeGetInfo(virConnectPtr conn, virNodeInfoPtr
nodeinfo)
++ptr;
}
+/* Make sure the string fits in mode */
+dynamicProperty->val->string[sizeof(nodeinfo->model) - 1] = '\0';
if (virStrcpyStatic(nodeinfo->model, dynamicProperty->val->string)
< 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("CPU Model %s too long for destination"),
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] esx: Fix build when libcurl debug is enabled
When building libvirt with libcurl debug enabled (with
ESX_VI__CURL__ENABLE_DEBUG_OUTPUT set), the message bellow pops up:
make[3]: Entering directory '/mnt/data/gitroot/libvirt/src'
CC esx/libvirt_driver_esx_la-esx_vi.lo
esx/esx_vi.c: In function 'esxVI_CURL_Debug':
esx/esx_vi.c:191:5: error: enumeration value 'CURLINFO_SSL_DATA_IN' not handled
in switch [-Werror=switch-enum]
switch (type) {
^~
esx/esx_vi.c:191:5: error: enumeration value 'CURLINFO_SSL_DATA_OUT' not
handled in switch [-Werror=switch-enum]
esx/esx_vi.c:191:5: error: enumeration value 'CURLINFO_END' not handled in
switch [-Werror=switch-enum]
Our build requires at least libcurl 7.18.0, which is pretty stable since
it was release in 2008. Fix this problem by handling the mentioned enums
in the code.
Signed-off-by: Marcos Paulo de Souza
---
src/esx/esx_vi.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index a816c3a4f9..588670e137 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -163,7 +163,7 @@ esxVI_CURL_WriteBuffer(char *data, size_t size, size_t
nmemb, void *userdata)
return 0;
}
-#define ESX_VI__CURL__ENABLE_DEBUG_OUTPUT 0
+#define ESX_VI__CURL__ENABLE_DEBUG_OUTPUT 1
#if ESX_VI__CURL__ENABLE_DEBUG_OUTPUT
static int
@@ -205,13 +205,19 @@ esxVI_CURL_Debug(CURL *curl ATTRIBUTE_UNUSED,
curl_infotype type,
break;
case CURLINFO_DATA_IN:
+ case CURLINFO_SSL_DATA_IN:
VIR_DEBUG("CURLINFO_DATA_IN %s", buffer);
break;
case CURLINFO_DATA_OUT:
+ case CURLINFO_SSL_DATA_OUT:
VIR_DEBUG("CURLINFO_DATA_OUT %s", buffer);
break;
+ case CURLINFO_END:
+VIR_DEBUG("CURLINFO_END %s", buffer);
+break;
+
default:
VIR_DEBUG("unknown");
break;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] esx:Fix esxDomainGetMaxVcpus to return correct vcpus
Before this change, esxDomainGetMaxVcpus returned -1, which in turn
fails in libvirt. This commit reimplements esxDomainGetMaxVcpus instead
of calling esxDomainGetVcpusFlags. The implementation checks for
capability.maxSupportedVcpus, but as this one can be ommited in ESXi, we
also check for capability.maxHostSupportedVcpus. With this change,
virDomainSetVcpus, virDomainGetMaxVcpus and virDomainGetVcpusFlags and
returning correct values.
Signed-off-by: Marcos Paulo de Souza
---
src/esx/esx_driver.c | 36 ++--
1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index d5e8a7b4eb..3169314fa4 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -2581,8 +2581,40 @@ esxDomainGetVcpusFlags(virDomainPtr domain, unsigned int
flags)
static int
esxDomainGetMaxVcpus(virDomainPtr domain)
{
-return esxDomainGetVcpusFlags(domain, (VIR_DOMAIN_AFFECT_LIVE |
- VIR_DOMAIN_VCPU_MAXIMUM));
+esxPrivate *priv = domain->conn->privateData;
+esxVI_String *propertyNameList = NULL;
+esxVI_ObjectContent *hostSystem = NULL;
+esxVI_Int *supportedVcpus = NULL;
+esxVI_Int *hostVcpus = NULL;
+
+if (esxVI_EnsureSession(priv->primary) < 0)
+return -1;
+
+priv->maxVcpus = -1;
+
+if (esxVI_String_AppendValueToList(,
+ "capability.maxHostSupportedVcpus\0"
+ "capability.maxSupportedVcpus"
+ ) < 0 ||
+esxVI_LookupHostSystemProperties(priv->primary, propertyNameList,
+ ) < 0 ||
+esxVI_GetInt(hostSystem, "capability.maxHostSupportedVcpus",
+ , esxVI_Occurrence_RequiredItem) < 0 ||
+esxVI_GetInt(hostSystem, "capability.maxSupportedVcpus",
+ , esxVI_Occurrence_OptionalItem) < 0)
+
+goto cleanup;
+
+/* as maxSupportedVcpus is optional, check also for maxHostSupportedVcpus
*/
+priv->maxVcpus = supportedVcpus ? supportedVcpus->value : hostVcpus->value;
+
+ cleanup:
+esxVI_String_Free();
+esxVI_ObjectContent_Free();
+esxVI_Int_Free();
+esxVI_Int_Free();
+
+return priv->maxVcpus;
}
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] esx: Make esxDomainGetVcpusFlags return vcpus again
Before this patch, esxDomainGetVcpusFlags was returning -1 since
"maxSupportedVcpus" can be NULL in ESXi[1]. In order to make it work,
replicate the same behavior than esxDomainGetInfo that used
config.hardware.numCPU to return the correct number of vcpus of a VM.
This patch, together with the next one, makes the calls
virDomainSetVcpus, virDomainGetMaxVcpus and virDomainGetVcpusFlags to
return successfull again.
[1]:https://pubs.vmware.com/vi-sdk/visdk250/ReferenceGuide/vim.host.Capability.html
Signed-off-by: Marcos Paulo de Souza
---
src/esx/esx_driver.c | 36 +++-
1 file changed, 11 insertions(+), 25 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index c2154799fa..d5e8a7b4eb 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -2547,45 +2547,31 @@ esxDomainGetVcpusFlags(virDomainPtr domain, unsigned
int flags)
{
esxPrivate *priv = domain->conn->privateData;
esxVI_String *propertyNameList = NULL;
-esxVI_ObjectContent *hostSystem = NULL;
-esxVI_DynamicProperty *dynamicProperty = NULL;
+esxVI_ObjectContent *virtualMachine = NULL;
+esxVI_Int *vcpus = NULL;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_VCPU_MAXIMUM, -1);
-if (priv->maxVcpus > 0)
-return priv->maxVcpus;
-
priv->maxVcpus = -1;
if (esxVI_EnsureSession(priv->primary) < 0)
return -1;
if (esxVI_String_AppendValueToList(,
- "capability.maxSupportedVcpus") < 0 ||
-esxVI_LookupHostSystemProperties(priv->primary, propertyNameList,
- ) < 0) {
+ "config.hardware.numCPU\0") < 0 ||
+esxVI_LookupVirtualMachineByUuid(priv->primary, domain->uuid,
+ propertyNameList, ,
+ esxVI_Occurrence_RequiredItem) < 0 ||
+ esxVI_GetInt(virtualMachine, "config.hardware.numCPU",
+ , esxVI_Occurrence_RequiredItem) < 0)
goto cleanup;
-}
-
-for (dynamicProperty = hostSystem->propSet; dynamicProperty;
- dynamicProperty = dynamicProperty->_next) {
-if (STREQ(dynamicProperty->name, "capability.maxSupportedVcpus")) {
-if (esxVI_AnyType_ExpectType(dynamicProperty->val,
- esxVI_Type_Int) < 0) {
-goto cleanup;
-}
-
-priv->maxVcpus = dynamicProperty->val->int32;
-break;
-} else {
-VIR_WARN("Unexpected '%s' property", dynamicProperty->name);
-}
-}
+priv->maxVcpus = vcpus->value;
cleanup:
esxVI_String_Free();
-esxVI_ObjectContent_Free();
+esxVI_ObjectContent_Free();
+esxVI_Int_Free();
return priv->maxVcpus;
}
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] esx: Fix {g,s}et vcpus
Hi guys, while checking my ESXi 6.5, and trying to query and change vcpus values, I discovered this was not working. So, these two patches aim to fix the problem. I checked some docs[1]. Let me know if there are points where it can be improved. Thanks, [1] https://www.vmware.com/support/orchestrator/doc/vco_vsphere55_api/html/VcHostCapability.html Marcos Paulo de Souza (2): esx: Make esxDomainGetVcpusFlags return vcpus again esx:Fix esxDomainGetMaxVcpus to return correct vcpus src/esx/esx_driver.c | 72 +++- 1 file changed, 45 insertions(+), 27 deletions(-) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] phyp: Drop check for auth and auth->cb
Since they are done inside virAuthGetPassword and virAuthGetUsername
when needed.
Signed-off-by: Marcos Paulo de Souza
---
src/phyp/phyp_driver.c | 11 ---
1 file changed, 11 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index d78de83231..5b17508dae 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -957,12 +957,6 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth,
if (VIR_STRDUP(username, conn->uri->user) < 0)
goto err;
} else {
-if (auth == NULL || auth->cb == NULL) {
-virReportError(VIR_ERR_AUTH_FAILED,
- "%s", _("No authentication callback provided."));
-goto err;
-}
-
username = virAuthGetUsername(conn, auth, "ssh", NULL,
conn->uri->server);
if (username == NULL) {
@@ -1039,11 +1033,6 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr
auth,
if (rc == LIBSSH2_ERROR_SOCKET_NONE
|| rc == LIBSSH2_ERROR_PUBLICKEY_UNRECOGNIZED
|| rc == LIBSSH2_ERROR_PUBLICKEY_UNVERIFIED) {
-if (auth == NULL || auth->cb == NULL) {
-virReportError(VIR_ERR_AUTH_FAILED,
- "%s", _("No authentication callback provided."));
-goto disconnect;
-}
password = virAuthGetPassword(conn, auth, "ssh", username,
conn->uri->server);
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/5] Unify check for auth and auth->cb
Hi guys, in some drivers, auth and auth->cb are checked in connectOpen function, while in xenapi, only auth is checked, which that could lead to a problem if cb if invalid. In phyp, auth and auth->cb are checked twice, in getUser and getPassword. So, this patchset adds the check for auth and auth->cb inside virAuthGetUsername and virAuthGetPassword, making it safer for all drivers that rely in auth callbacks. Marcos Paulo de Souza (5): virauth.c: Check for valid auth callback esx: Drop check for auth and auth->cb hyperv: Drop check for auth and auth->cb phyp: Drop check for auth and auth->cb xenapi: Drop check for auth src/esx/esx_driver.c | 7 --- src/hyperv/hyperv_driver.c | 7 --- src/phyp/phyp_driver.c | 11 --- src/util/virauth.c | 12 src/xenapi/xenapi_driver.c | 6 -- 5 files changed, 12 insertions(+), 31 deletions(-) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] hyperv: Drop check for auth and auth->cb
Since they are done inside virAuthGetPassword and virAuthGetUsername
when needed.
Signed-off-by: Marcos Paulo de Souza
---
src/hyperv/hyperv_driver.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 6bc4c099e2..7ca145aef5 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -128,13 +128,6 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr
auth,
virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
-/* Require auth */
-if (auth == NULL || auth->cb == NULL) {
-virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("Missing or invalid auth pointer"));
-return VIR_DRV_OPEN_ERROR;
-}
-
/* Allocate per-connection private data */
if (VIR_ALLOC(priv) < 0)
goto cleanup;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/2] esx: Do not crash SetAutoStart by double free
SetAutoStart method cannot free virtualMachine using esxVI_ObjectContent_Free, since: esxVI_HostAutoStartManagerConfig_Free -> esxVI_AutoStartPowerInfo_Free -> esxVI_ManagedObjectReference_Free(item->key); item->key, in this context, is virtualMachine->obj, so calling esxVI_ObjectContent_Free creates a double free, becasuse esxVI_ObjectContent_Free also calls esxVI_ManagedObjectReference_Free(>obj). Removing the esxVI_ObjectContent_Free from SetAutoStart fixes this problem. Signed-off-by: Marcos Paulo de Souza --- src/esx/esx_driver.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index cee98ebcaf..3835e4cb3c 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -3421,7 +3421,9 @@ esxDomainSetAutostart(virDomainPtr domain, int autostart) newPowerInfo->stopAction = NULL; } -esxVI_ObjectContent_Free(); +/* HostAutoStartManagerConfig free method will call autoStartPowerInfoFree + * in order to free virtualMachine, since newPowerInfo-> key points to + * virtualMachine */ esxVI_HostAutoStartManagerConfig_Free(); esxVI_AutoStartDefaults_Free(); esxVI_AutoStartPowerInfo_Free(); -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/2] ESX: Fixing SetAutoStart
On Thu, Aug 02, 2018 at 09:23:05PM +0200, Matthias Bolte wrote:
> 2018-08-02 18:16 GMT+02:00 John Ferlan :
> >
> >
> > On 08/02/2018 12:11 PM, Marcos Paulo de Souza wrote:
> >> On Thu, Aug 02, 2018 at 05:37:46PM +0200, Matthias Bolte wrote:
> >>> 2018-08-02 16:45 GMT+02:00 John Ferlan :
> >>>>
> >>>>
> >>>> On 08/02/2018 10:07 AM, Matthias Bolte wrote:
> >>>>> 2018-08-02 15:20 GMT+02:00 John Ferlan :
> >>>>>>
> >>>>>>
> >>>>>> On 08/02/2018 05:04 AM, Matthias Bolte wrote:
> >>>>>>> 2018-08-01 18:09 GMT+02:00 Marcos Paulo de Souza
> >>>>>>> :
> >>>>>>>> This is a new version from the last patchset sent yesterday, but now
> >>>>>>>> using
> >>>>>>>> VIR_STRNDUP, instead of allocating memory manually.
> >>>>>>>>
> >>>>>>>> First version:
> >>>>>>>> https://www.redhat.com/archives/libvir-list/2018-August/msg0.html
> >>>>>>>>
> >>>>>>>> Marcos Paulo de Souza (2):
> >>>>>>>> esx: Do not crash SetAutoStart by double free
> >>>>>>>> esx: Fix SetAutoStart invalid pointer free
> >>>>>>>>
> >>>>>>>> src/esx/esx_driver.c | 14 +++---
> >>>>>>>> 1 file changed, 11 insertions(+), 3 deletions(-)
> >>>>>>>
> >>>>>>> I see the problem, but your approach is too complicated.
> >>>>>>>
> >>>>>>> There is already code in place to handle those situations:
> >>>>>>>
> >>>>>>> 3417 cleanup:
> >>>>>>> 3418 if (newPowerInfo) {
> >>>>>>> 3419 newPowerInfo->key = NULL;
> >>>>>>> 3420 newPowerInfo->startAction = NULL;
> >>>>>>> 3421 newPowerInfo->stopAction = NULL;
> >>>>>>> 3422 }
> >>>>>>>
> >>>>>>> That resets those fields to NULL to avoid double freeing and freeing
> >>>>>>> static strings.
> >>>>>>>
> >>>>>>> The problem is that commit ae83e02f3dd7fe99fed5d8159a35b666fafeafd5 by
> >>>>>>> John Frelan broke this logic, by setting newPowerInfo to NULL in the
> >>>>>>> success path, to silence Coverity.
> >>>>>>>
> >>>>>>
> >>>>>> Memory recall from "Tue Sep 3 07:20:34 2013 -0400" is perhaps
> >>>>>> impossible
> >>>>>> ;-)... TL;DR, looks like the error back then was a false positive
> >>>>>> because (as I know I've learned since then) Coverity has a hard time
> >>>>>> when a boolean or size_t count is used to control whether or not memory
> >>>>>> would be freed.
> >>>>>>
> >>>>>> Looking at the logic:
> >>>>>>
> >>>>>> if (esxVI_AutoStartPowerInfo_AppendToList(>powerInfo,
> >>>>>> newPowerInfo) < 0) {
> >>>>>> goto cleanup;
> >>>>>> }
> >>>>>>
> >>>>>> newPowerInfo = NULL;
> >>>>>>
> >>>>>> and following it to esxVI_List_Append which on success would seemingly
> >>>>>> assign @newPowerInfo into the >powerInfo list, I can certainly
> >>>>>> see
> >>>>>> why logically setting newPowerInfo = NULL after than would seem to be
> >>>>>> the right thing. Of course, I see now the subtle reason why it's not a
> >>>>>> good idea.
> >>>>>>
> >>>>>> Restoring logic from that commit in esxDomainSetAutostart, then
> >>>>>> Coverity
> >>>>>> believes that @newPowerInfo is leaked from the goto cleanup at
> >>>>>> allocation because for some reason it has decided it must evaluate both
> >>>>>> true and false of a condition even though the logic only ever set the
> >>>>>> boolean when @newPowerInfo was placed onto the @spec->powerInfo list.
> >>>&
[libvirt] [PATCH 1/5] virauth.c: Check for valid auth callback
Instead of adding the same check for every drivers, execute the checks
in virAuthGetUsername and virAuthGetPassword. These funtions are called
when user is not set in the URI.
Signed-off-by: Marcos Paulo de Souza
---
src/util/virauth.c | 12
1 file changed, 12 insertions(+)
diff --git a/src/util/virauth.c b/src/util/virauth.c
index 8c450b6b31..759b8f0cd3 100644
--- a/src/util/virauth.c
+++ b/src/util/virauth.c
@@ -198,6 +198,12 @@ virAuthGetUsername(virConnectPtr conn,
if (virAuthGetConfigFilePath(conn, ) < 0)
return NULL;
+if (!auth || !auth->cb) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("Missing or invalid auth pointer"));
+return NULL;
+}
+
return virAuthGetUsernamePath(path, auth, servicename,
defaultUsername, hostname);
}
@@ -262,5 +268,11 @@ virAuthGetPassword(virConnectPtr conn,
if (virAuthGetConfigFilePath(conn, ) < 0)
return NULL;
+if (!auth || !auth->cb) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("Missing or invalid auth pointer"));
+return NULL;
+}
+
return virAuthGetPasswordPath(path, auth, servicename, username, hostname);
}
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] esx: Drop check for auth and auth->cb
Since they are done inside virAuthGetPassword and virAuthGetUsername
when needed.
Signed-off-by: Marcos Paulo de Souza
---
src/esx/esx_driver.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index c2154799fa..15785858c6 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -854,13 +854,6 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
conn->uri->path, conn->uri->scheme);
}
-/* Require auth */
-if (!auth || !auth->cb) {
-virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("Missing or invalid auth pointer"));
-return VIR_DRV_OPEN_ERROR;
-}
-
/* Allocate per-connection private data */
if (VIR_ALLOC(priv) < 0)
goto cleanup;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] xenapi: Drop check for auth
Since they are done inside virAuthGetPassword and virAuthGetUsername
when needed. Also, only auth is checked, but auth->cb, which that could
lead to a crash if the callback is NULL.
Signed-off-by: Marcos Paulo de Souza
---
src/xenapi/xenapi_driver.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
index 34f9e2c717..3af5eeafcf 100644
--- a/src/xenapi/xenapi_driver.c
+++ b/src/xenapi/xenapi_driver.c
@@ -152,12 +152,6 @@ xenapiConnectOpen(virConnectPtr conn, virConnectAuthPtr
auth,
goto error;
}
-if (auth == NULL) {
-xenapiSessionErrorHandler(conn, VIR_ERR_AUTH_FAILED,
- _("Authentication Credentials not found"));
-goto error;
-}
-
if (conn->uri->user != NULL) {
if (VIR_STRDUP(username, conn->uri->user) < 0)
goto error;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/2] ESX: Fixing SetAutoStart
On Thu, Aug 02, 2018 at 05:37:46PM +0200, Matthias Bolte wrote:
> 2018-08-02 16:45 GMT+02:00 John Ferlan :
> >
> >
> > On 08/02/2018 10:07 AM, Matthias Bolte wrote:
> >> 2018-08-02 15:20 GMT+02:00 John Ferlan :
> >>>
> >>>
> >>> On 08/02/2018 05:04 AM, Matthias Bolte wrote:
> >>>> 2018-08-01 18:09 GMT+02:00 Marcos Paulo de Souza
> >>>> :
> >>>>> This is a new version from the last patchset sent yesterday, but now
> >>>>> using
> >>>>> VIR_STRNDUP, instead of allocating memory manually.
> >>>>>
> >>>>> First version:
> >>>>> https://www.redhat.com/archives/libvir-list/2018-August/msg0.html
> >>>>>
> >>>>> Marcos Paulo de Souza (2):
> >>>>> esx: Do not crash SetAutoStart by double free
> >>>>> esx: Fix SetAutoStart invalid pointer free
> >>>>>
> >>>>> src/esx/esx_driver.c | 14 +++---
> >>>>> 1 file changed, 11 insertions(+), 3 deletions(-)
> >>>>
> >>>> I see the problem, but your approach is too complicated.
> >>>>
> >>>> There is already code in place to handle those situations:
> >>>>
> >>>> 3417 cleanup:
> >>>> 3418 if (newPowerInfo) {
> >>>> 3419 newPowerInfo->key = NULL;
> >>>> 3420 newPowerInfo->startAction = NULL;
> >>>> 3421 newPowerInfo->stopAction = NULL;
> >>>> 3422 }
> >>>>
> >>>> That resets those fields to NULL to avoid double freeing and freeing
> >>>> static strings.
> >>>>
> >>>> The problem is that commit ae83e02f3dd7fe99fed5d8159a35b666fafeafd5 by
> >>>> John Frelan broke this logic, by setting newPowerInfo to NULL in the
> >>>> success path, to silence Coverity.
> >>>>
> >>>
> >>> Memory recall from "Tue Sep 3 07:20:34 2013 -0400" is perhaps impossible
> >>> ;-)... TL;DR, looks like the error back then was a false positive
> >>> because (as I know I've learned since then) Coverity has a hard time
> >>> when a boolean or size_t count is used to control whether or not memory
> >>> would be freed.
> >>>
> >>> Looking at the logic:
> >>>
> >>> if (esxVI_AutoStartPowerInfo_AppendToList(>powerInfo,
> >>> newPowerInfo) < 0) {
> >>> goto cleanup;
> >>> }
> >>>
> >>> newPowerInfo = NULL;
> >>>
> >>> and following it to esxVI_List_Append which on success would seemingly
> >>> assign @newPowerInfo into the >powerInfo list, I can certainly see
> >>> why logically setting newPowerInfo = NULL after than would seem to be
> >>> the right thing. Of course, I see now the subtle reason why it's not a
> >>> good idea.
> >>>
> >>> Restoring logic from that commit in esxDomainSetAutostart, then Coverity
> >>> believes that @newPowerInfo is leaked from the goto cleanup at
> >>> allocation because for some reason it has decided it must evaluate both
> >>> true and false of a condition even though the logic only ever set the
> >>> boolean when @newPowerInfo was placed onto the @spec->powerInfo list.
> >>> IOW: A false positive because the human can read the code and say that
> >>> Coverity is full of it.
> >>>
> >>> So either I add this to my Coverity list of false positives or in the
> >>> "if (esxVI_AutoStartPowerInfo_Alloc() < 0 || " condition
> >>> cleanup logic call esxVI_AutoStartPowerInfo_Free(); prior
> >>> to cleanup, removing it from the cleanup: path, and then remove the
> >>> "newPowerInfo = NULL;" after list insertion.
> >>>
> >>>
> >>>
> >>> John
> >>
> >> Okay, I see what's going on. I suggest this alternative patch that
> >> keeps the newPowerInfo = NULL line to make Coverity understand the
> >> cleanup code, but adds a second variable to restore the original
> >> logic. I hope this doesn't upset Coverity.
> >>
> >> Marcos, can you give the attached patch a try? It should fix the
> >> problems you try to fix here, by restoring the original cleanup logic.
> >>
> >
> &
[libvirt] [PATCH v2 0/2] ESX: Fixing SetAutoStart
This is a new version from the last patchset sent yesterday, but now using VIR_STRNDUP, instead of allocating memory manually. First version: https://www.redhat.com/archives/libvir-list/2018-August/msg0.html Marcos Paulo de Souza (2): esx: Do not crash SetAutoStart by double free esx: Fix SetAutoStart invalid pointer free src/esx/esx_driver.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/2] esx: Fix SetAutoStart invalid pointer free
esxVI_AutoStartPowerInfo_Free, which is called from
esxVI_HostAutoStartManagerConfig_Free, will always call VIR_FREE to free
memory from {start,stop}Action, leading to a invalid pointer.
With this patch applied, ESX can set autostart successfully to all it's
domains.
Signed-off-by: Marcos Paulo de Souza
---
Changes from v1:
* Stop calling VIR_ALLOC_N and strcpy, and use VIR_STRNDUP instead
src/esx/esx_driver.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 3835e4cb3c..dc07cf8770 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -3394,9 +3394,15 @@ esxDomainSetAutostart(virDomainPtr domain, int autostart)
newPowerInfo->startOrder->value = -1; /* no specific start order */
newPowerInfo->startDelay->value = -1; /* use system default */
newPowerInfo->waitForHeartbeat =
esxVI_AutoStartWaitHeartbeatSetting_SystemDefault;
-newPowerInfo->startAction = autostart ? (char *)"powerOn" : (char *)"none";
newPowerInfo->stopDelay->value = -1; /* use system default */
-newPowerInfo->stopAction = (char *)"none";
+
+/* startAction and stopAction will be freed by
esxVI_HostAutoStartManagerConfig_Free */
+if (VIR_STRNDUP(newPowerInfo->startAction, autostart ? "powerOn" : "none",
+autostart ? 7 : 4) < 1)
+goto cleanup;
+
+if (VIR_STRNDUP(newPowerInfo->stopAction, "none", 4) < 1)
+goto cleanup;
if (esxVI_AutoStartPowerInfo_AppendToList(>powerInfo,
newPowerInfo) < 0) {
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] esx: Fix SetAutoStart invalid pointer free
esxVI_AutoStartPowerInfo_Free, which is called from
esxVI_HostAutoStartManagerConfig_Free, will always call VIR_FREE to free
memory from {start,stop}Action, leading to a invalid pointer.
With this patch applied, ESX can set autostart successfully to all it's
domains.
Signed-off-by: Marcos Paulo de Souza
---
src/esx/esx_driver.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 3835e4cb3c..a49862a1de 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -3386,7 +3386,9 @@ esxDomainSetAutostart(virDomainPtr domain, int autostart)
if (esxVI_AutoStartPowerInfo_Alloc() < 0 ||
esxVI_Int_Alloc(>startOrder) < 0 ||
esxVI_Int_Alloc(>startDelay) < 0 ||
-esxVI_Int_Alloc(>stopDelay) < 0) {
+esxVI_Int_Alloc(>stopDelay) < 0 ||
+VIR_ALLOC_N(newPowerInfo->startAction, 8) < 0 ||
+VIR_ALLOC_N(newPowerInfo->stopAction, 5) < 0) {
goto cleanup;
}
@@ -3394,9 +3396,9 @@ esxDomainSetAutostart(virDomainPtr domain, int autostart)
newPowerInfo->startOrder->value = -1; /* no specific start order */
newPowerInfo->startDelay->value = -1; /* use system default */
newPowerInfo->waitForHeartbeat =
esxVI_AutoStartWaitHeartbeatSetting_SystemDefault;
-newPowerInfo->startAction = autostart ? (char *)"powerOn" : (char *)"none";
+strcpy(newPowerInfo->startAction, autostart ? (char *)"powerOn" : (char
*)"none");
newPowerInfo->stopDelay->value = -1; /* use system default */
-newPowerInfo->stopAction = (char *)"none";
+strcpy(newPowerInfo->stopAction, (char *)"none");
if (esxVI_AutoStartPowerInfo_AppendToList(>powerInfo,
newPowerInfo) < 0) {
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] esx: Do not crash SetAutoStart by double free
SetAutoStart method cannot free virtualMachine using esxVI_ObjectContent_Free, since: esxVI_HostAutoStartManagerConfig_Free -> esxVI_AutoStartPowerInfo_Free -> esxVI_ManagedObjectReference_Free(item->key); item->key, in this context, is virtualMachine->obj, so calling esxVI_ObjectContent_Free creates a double free, becasuse esxVI_ObjectContent_Free also calls esxVI_ManagedObjectReference_Free(>obj). Removing the esxVI_ObjectContent_Free from SetAutoStart fixes this problem. Signed-off-by: Marcos Paulo de Souza --- src/esx/esx_driver.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index cee98ebcaf..3835e4cb3c 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -3421,7 +3421,9 @@ esxDomainSetAutostart(virDomainPtr domain, int autostart) newPowerInfo->stopAction = NULL; } -esxVI_ObjectContent_Free(); +/* HostAutoStartManagerConfig free method will call autoStartPowerInfoFree + * in order to free virtualMachine, since newPowerInfo-> key points to + * virtualMachine */ esxVI_HostAutoStartManagerConfig_Free(); esxVI_AutoStartDefaults_Free(); esxVI_AutoStartPowerInfo_Free(); -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 0/2] ESX: Fixing SetAutoStart
Hi guys, while doing some random tests using the ESX driver, I faced a crash, and these two patches are trying to solve. I don't know if there is a better way to solve it, since ESX driver *generated* code to alloc and free some *generated* structs, but, please let me know if you have a better idea. Thanks, Marcos Paulo de Souza (2): esx: Do not crash SetAutoStart by double free esx: Fix SetAutoStart invalid pointer free src/esx/esx_driver.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] esx storage: Fix typo lsilogic -> lsiLogic
Commit 77298458d027db4d3e082213355e2d792f65158d changed the esx storage adapter from busLogic to lsilogic, introducing a typo. Changing it back to lsiLogic (with capital L) solves the issue. With this change, libvirt can now create volumes in ESX again. Thanks to Jaroslav Suchanek who figured out what was the issue in the first place. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1571759 Signed-off-by: Marcos Paulo de Souza --- src/esx/esx_storage_backend_vmfs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index 630a6aa8c9..bb2de4b69f 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -967,9 +967,9 @@ esxStorageVolCreateXML(virStoragePoolPtr pool, /* * FIXME: The adapter type is a required parameter, but there is no * way to let the user specify it in the volume XML config. Therefore, - * default to 'lsilogic' here. + * default to 'lsiLogic' here. */ -virtualDiskSpec->adapterType = (char *)"lsilogic"; +virtualDiskSpec->adapterType = (char *)"lsiLogic"; virtualDiskSpec->capacityKb->value = VIR_DIV_UP(def->target.capacity, 1024); /* Scale from byte to kilobyte */ -- 2.17.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 2/5] libvirt.c: Return error when remoteOnly is set but server is empty
Some drivers require a server in order to work, so this flag removes the
burden of esach driver to check for an server by doing it in
virConnectOpenInternal.
Signed-off-by: Marcos Paulo de Souza
---
src/libvirt.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/src/libvirt.c b/src/libvirt.c
index 52f4dd2808..aeb8804714 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1029,6 +1029,13 @@ virConnectOpenInternal(const char *name,
VIR_DEBUG("Matching any URI scheme for '%s'", ret->uri ?
ret->uri->scheme : "");
}
+/* before starting the new connection, check if the driver only works
+ * with a server, and so return an error if the server is missing */
+if (virConnectDriverTab[i]->remoteOnly && ret->uri &&
!ret->uri->server) {
+virReportError(VIR_ERR_INVALID_ARG, "%s", _("URI is missing the
server part"));
+goto failed;
+}
+
ret->driver = virConnectDriverTab[i]->hypervisorDriver;
ret->interfaceDriver = virConnectDriverTab[i]->interfaceDriver;
ret->networkDriver = virConnectDriverTab[i]->networkDriver;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 5/5] phyp_driver: Set remoteOnly member of virConnectDriver
Phyp driver can't function without a server being informed, so this flag
makes libvirt to check for a valid server before calling connectOpen.
Signed-off-by: Marcos Paulo de Souza
---
src/phyp/phyp_driver.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 67ce7903ba..303a7151b7 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -1141,12 +1141,6 @@ phypConnectOpen(virConnectPtr conn,
virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
-if (conn->uri->server == NULL) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("Missing server name in phyp:// URI"));
-return VIR_DRV_OPEN_ERROR;
-}
-
if (VIR_ALLOC(phyp_driver) < 0)
goto failure;
@@ -3764,6 +3758,7 @@ static virConnectDriver phypConnectDriver = {
.hypervisorDriver = ,
.interfaceDriver = ,
.storageDriver = ,
+.remoteOnly = true,
};
int
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 0/5] Making libvirt aware of a server need
Hi guys, this is the third version of the patchset to add a new member to virConnectDriver in order to simplfy the check for a server. Now, libvirt does the work inside virConnectOpenInternal after getting the correct driver from the URI. v2: https://www.redhat.com/archives/libvir-list/2018-July/msg00501.html v1: https://www.redhat.com/archives/libvir-list/2018-July/msg00393.html Changes from v2: * patch 2: Move check for remoteOnly member just before calling connectOpen * (only patch 2 changed in v2) Changes from v1: * Adapted the code following ideas from Matthias Bolte Marcos Paulo de Souza (5): driver.h: Add remoteOnly member to virConnectDriver struct libvirt.c: Return error when remoteOnly is set but server is empty esx_driver: Set remoteOnly member of virConnectDriver hyperv_driver: Set remoteOnly member of virConnectDriver phyp_driver: Set remoteOnly member of virConnectDriver src/driver.h | 2 ++ src/esx/esx_driver.c | 8 +--- src/hyperv/hyperv_driver.c | 8 +--- src/libvirt.c | 7 +++ src/phyp/phyp_driver.c | 7 +-- 5 files changed, 12 insertions(+), 20 deletions(-) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 3/5] esx_driver: Set remoteOnly member of virConnectDriver
ESX driver can't function without a server being informed, so this flag
makes libvirt to check for a valid server before calling connectOpen.
Signed-off-by: Marcos Paulo de Souza
---
src/esx/esx_driver.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 06e1238385..d937daac83 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -854,13 +854,6 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
conn->uri->path, conn->uri->scheme);
}
-/* Require server part */
-if (!conn->uri->server) {
-virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("URI is missing the server part"));
-return VIR_DRV_OPEN_ERROR;
-}
-
/* Require auth */
if (!auth || !auth->cb) {
virReportError(VIR_ERR_INVALID_ARG, "%s",
@@ -5218,6 +5211,7 @@ static virConnectDriver esxConnectDriver = {
.interfaceDriver = ,
.networkDriver = ,
.storageDriver = ,
+.remoteOnly = true,
};
int
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 1/5] driver.h: Add remoteOnly member to virConnectDriver struct
This new flag will be set when a driver needs a remote URL in order to
work, as ESX, HyperV and Phyp.
Signed-off-by: Marcos Paulo de Souza
---
src/driver.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/driver.h b/src/driver.h
index 0b1f7a2269..c4d94ba294 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -81,6 +81,8 @@ typedef virConnectDriver *virConnectDriverPtr;
struct _virConnectDriver {
/* Wether driver permits a server in the URI */
bool localOnly;
+/* Wether driver needs a server in the URI */
+bool remoteOnly;
/*
* NULL terminated list of supported URI schemes.
* - Single element { NULL } list indicates no supported schemes
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 4/5] hyperv_driver: Set remoteOnly member of virConnectDriver
HyperV driver can't function without a server being informed, so this flag
makes libvirt to check for a valid server before calling connectOpen.
Signed-off-by: Marcos Paulo de Souza
---
src/hyperv/hyperv_driver.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 95496bdf73..8bda334cf9 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -128,13 +128,6 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr
auth,
virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
-/* Require server part */
-if (conn->uri->server == NULL) {
-virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("URI is missing the server part"));
-return VIR_DRV_OPEN_ERROR;
-}
-
/* Require auth */
if (auth == NULL || auth->cb == NULL) {
virReportError(VIR_ERR_INVALID_ARG, "%s",
@@ -1664,6 +1657,7 @@ hypervDebugHandler(const char *message, debug_level_e
level,
static virConnectDriver hypervConnectDriver = {
.uriSchemes = (const char *[]){ "hyperv", NULL },
.hypervisorDriver = ,
+.remoteOnly = true,
};
int
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] Making libvirt aware of a server need
On Mon, Jul 09, 2018 at 01:22:18PM -0300, Marcos Paulo de Souza wrote: > Hi guys, > > this is basically a second version of [1], but I rewrote the patch following > Matthias ideas [2]. It really seems a cleaner change, but, feel free to > suggest > any other change that could make the code better. > > I did a lot of tests using virsh tool, so when using the ESX driver without a > proper server, it returns a error. I could not test hyperv and phyp. > > Thanks, > > [1]: https://www.redhat.com/archives/libvir-list/2018-July/msg00393.html > [2]: https://www.redhat.com/archives/libvir-list/2018-July/msg00400.html This patchset is still not ready, I found a problem in my approach in libvirt.c. Will resent a v3 once this is fixed. Thanks, > > Marcos Paulo de Souza (5): > driver.h: Add remoteOnly member to virConnectDriver struct > libvirt.c: Return error when remoteOnly is set but server is empty > esx_driver: Set remoteOnly member of virConnectDriver > hyperv_driver: Set remoteOnly member of virConnectDriver > phyp_driver: Set remoteOnly member of virConnectDriver > > src/driver.h | 2 ++ > src/esx/esx_driver.c | 8 +--- > src/hyperv/hyperv_driver.c | 8 +--- > src/libvirt.c | 5 + > src/phyp/phyp_driver.c | 7 +-- > 5 files changed, 10 insertions(+), 20 deletions(-) > > -- > 2.17.1 > -- Thanks, Marcos -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] hyperv_driver: Set remoteOnly member of virConnectDriver
HyperV driver can't function without a server being informed, so this flag
makes libvirt to check for a valid server before calling connectOpen.
Signed-off-by: Marcos Paulo de Souza
---
src/hyperv/hyperv_driver.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 95496bdf73..8bda334cf9 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -128,13 +128,6 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr
auth,
virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
-/* Require server part */
-if (conn->uri->server == NULL) {
-virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("URI is missing the server part"));
-return VIR_DRV_OPEN_ERROR;
-}
-
/* Require auth */
if (auth == NULL || auth->cb == NULL) {
virReportError(VIR_ERR_INVALID_ARG, "%s",
@@ -1664,6 +1657,7 @@ hypervDebugHandler(const char *message, debug_level_e
level,
static virConnectDriver hypervConnectDriver = {
.uriSchemes = (const char *[]){ "hyperv", NULL },
.hypervisorDriver = ,
+.remoteOnly = true,
};
int
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] phyp_driver: Set remoteOnly member of virConnectDriver
Phyp driver can't function without a server being informed, so this flag
makes libvirt to check for a valid server before calling connectOpen.
Signed-off-by: Marcos Paulo de Souza
---
src/phyp/phyp_driver.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 67ce7903ba..303a7151b7 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -1141,12 +1141,6 @@ phypConnectOpen(virConnectPtr conn,
virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
-if (conn->uri->server == NULL) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("Missing server name in phyp:// URI"));
-return VIR_DRV_OPEN_ERROR;
-}
-
if (VIR_ALLOC(phyp_driver) < 0)
goto failure;
@@ -3764,6 +3758,7 @@ static virConnectDriver phypConnectDriver = {
.hypervisorDriver = ,
.interfaceDriver = ,
.storageDriver = ,
+.remoteOnly = true,
};
int
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] driver.h: Add remoteOnly member to virConnectDriver struct
This new flag will be set when a driver needs a remote URL in order to
work, as ESX, HyperV and Phyp.
Signed-off-by: Marcos Paulo de Souza
---
src/driver.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/driver.h b/src/driver.h
index 0b1f7a2269..c4d94ba294 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -81,6 +81,8 @@ typedef virConnectDriver *virConnectDriverPtr;
struct _virConnectDriver {
/* Wether driver permits a server in the URI */
bool localOnly;
+/* Wether driver needs a server in the URI */
+bool remoteOnly;
/*
* NULL terminated list of supported URI schemes.
* - Single element { NULL } list indicates no supported schemes
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/5] Making libvirt aware of a server need
Hi guys, this is basically a second version of [1], but I rewrote the patch following Matthias ideas [2]. It really seems a cleaner change, but, feel free to suggest any other change that could make the code better. I did a lot of tests using virsh tool, so when using the ESX driver without a proper server, it returns a error. I could not test hyperv and phyp. Thanks, [1]: https://www.redhat.com/archives/libvir-list/2018-July/msg00393.html [2]: https://www.redhat.com/archives/libvir-list/2018-July/msg00400.html Marcos Paulo de Souza (5): driver.h: Add remoteOnly member to virConnectDriver struct libvirt.c: Return error when remoteOnly is set but server is empty esx_driver: Set remoteOnly member of virConnectDriver hyperv_driver: Set remoteOnly member of virConnectDriver phyp_driver: Set remoteOnly member of virConnectDriver src/driver.h | 2 ++ src/esx/esx_driver.c | 8 +--- src/hyperv/hyperv_driver.c | 8 +--- src/libvirt.c | 5 + src/phyp/phyp_driver.c | 7 +-- 5 files changed, 10 insertions(+), 20 deletions(-) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] esx_driver: Set remoteOnly member of virConnectDriver
ESX driver can't function without a server being informed, so this flag
makes libvirt to check for a valid server before calling connectOpen.
Signed-off-by: Marcos Paulo de Souza
---
src/esx/esx_driver.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 06e1238385..d937daac83 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -854,13 +854,6 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
conn->uri->path, conn->uri->scheme);
}
-/* Require server part */
-if (!conn->uri->server) {
-virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("URI is missing the server part"));
-return VIR_DRV_OPEN_ERROR;
-}
-
/* Require auth */
if (!auth || !auth->cb) {
virReportError(VIR_ERR_INVALID_ARG, "%s",
@@ -5218,6 +5211,7 @@ static virConnectDriver esxConnectDriver = {
.interfaceDriver = ,
.networkDriver = ,
.storageDriver = ,
+.remoteOnly = true,
};
int
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] libvirt.c: Return error when remoteOnly is set but server is empty
Some drivers require a server in order to work, so this flag removes the
burden of esach driver to check for an server by doing it in
virConnectOpenInternal.
Signed-off-by: Marcos Paulo de Souza
---
src/libvirt.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/src/libvirt.c b/src/libvirt.c
index 52f4dd2808..9ae4e774eb 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1005,6 +1005,11 @@ virConnectOpenInternal(const char *name,
continue;
}
+if (virConnectDriverTab[i]->remoteOnly && ret->uri &&
!ret->uri->server) {
+virReportError(VIR_ERR_INVALID_ARG, "%s", _("URI is missing the
server part"));
+goto failed;
+}
+
/* Filter drivers based on declared URI schemes */
if (virConnectDriverTab[i]->uriSchemes) {
bool matchScheme = false;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH 1/4] driver.h: Add macro VIR_DRV_CONN_CHECK_SERVER
On Mon, Jul 09, 2018 at 08:31:05AM +0200, Peter Krempa wrote:
> On Sat, Jul 07, 2018 at 22:06:53 -0300, Marcos Paulo de Souza wrote:
> > This new macro will check if the server was passed to connectOpen
> > function. Some drivers as esx, hyperv and phyp need a server address to
> > connect to.
> >
> > Signed-off-by: Marcos Paulo de Souza
> > ---
> > src/driver.h | 9 +
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/src/driver.h b/src/driver.h
> > index 0b1f7a2269..1f9d7fd26b 100644
> > --- a/src/driver.h
> > +++ b/src/driver.h
> > @@ -60,6 +60,15 @@ typedef enum {
> > ((drv)->connectSupportsFeature ? \
> > (drv)->connectSupportsFeature((conn), (feature)) > 0 : 0)
> >
> > +/* Check if the connection of the driver has filled the server address */
> > +# define VIR_DRV_CONN_CHECK_SERVER \
>
> 'conn' should be passed in as an argument. Without that it is less
> obvious what is happening here.
>
> P.S: Yes I'm aware that we have e.g. virCheckFlags that checks the
> 'flags' variable in any context, but we don't have to introduce more of
> it.
>
How about to follow Matthias idea, and add a new member to virConnectDriver?
(https://www.redhat.com/archives/libvir-list/2018-July/msg00400.html)
I am already preparing a new patchset following his idea.
--
Thanks,
Marcos
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] examples: Remove config.h from hellolibvirt and openauth
These two examples do not use any definition from config.h, and by removing it these examples can be compiled standalone. Signed-off-by: Marcos Paulo de Souza --- When trying to remove config.h from domtop and domsuspend, a later inclusion of unistd.h complains about the missing config.h. This unistd.h come from gnulib, so I need more time to check why this header would complain, so for now just these examples are a good start ... examples/hellolibvirt/hellolibvirt.c | 2 -- examples/openauth/openauth.c | 2 -- 2 files changed, 4 deletions(-) diff --git a/examples/hellolibvirt/hellolibvirt.c b/examples/hellolibvirt/hellolibvirt.c index 02c4401987..bfb12846e6 100644 --- a/examples/hellolibvirt/hellolibvirt.c +++ b/examples/hellolibvirt/hellolibvirt.c @@ -2,8 +2,6 @@ * hypervisor and gather a few bits of information about domains. * Similar API's exist for storage pools, networks, and interfaces. */ -#include - #include #include #include diff --git a/examples/openauth/openauth.c b/examples/openauth/openauth.c index eef46d5f52..efd21c374f 100644 --- a/examples/openauth/openauth.c +++ b/examples/openauth/openauth.c @@ -1,8 +1,6 @@ /* This is a copy of the hellolibvirt example demonstaring how to use * virConnectOpenAuth with a custom auth callback */ -#include - #include #include #include -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH 1/4] driver.h: Add macro VIR_DRV_CONN_CHECK_SERVER
This new macro will check if the server was passed to connectOpen
function. Some drivers as esx, hyperv and phyp need a server address to
connect to.
Signed-off-by: Marcos Paulo de Souza
---
src/driver.h | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/driver.h b/src/driver.h
index 0b1f7a2269..1f9d7fd26b 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -60,6 +60,15 @@ typedef enum {
((drv)->connectSupportsFeature ? \
(drv)->connectSupportsFeature((conn), (feature)) > 0 : 0)
+/* Check if the connection of the driver has filled the server address */
+# define VIR_DRV_CONN_CHECK_SERVER \
+do { \
+if (!conn->uri->server) { \
+virReportError(VIR_ERR_INVALID_ARG, "%s", \
+ _("URI is missing the server part")); \
+return VIR_DRV_OPEN_ERROR; \
+} \
+} while (0)
# define __VIR_DRIVER_H_INCLUDES___
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH 2/4] esx_driver: Use VIR_DRV_CONN_CHECK_SERVER macro
ESX needs a server to connect to, so use this macro in order to reuse
code.
Signed-off-by: Marcos Paulo de Souza
---
src/esx/esx_driver.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 06e1238385..89ac7c430f 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -854,12 +854,7 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
conn->uri->path, conn->uri->scheme);
}
-/* Require server part */
-if (!conn->uri->server) {
-virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("URI is missing the server part"));
-return VIR_DRV_OPEN_ERROR;
-}
+VIR_DRV_CONN_CHECK_SERVER;
/* Require auth */
if (!auth || !auth->cb) {
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH 0/4] Add new macro to check for existent server
Hello, I am trying to add a generic way to check for server, specially in drivers that needs the server to be declared. If there is better way to put the check, or a better way to check this setting, let me know. Thanks in advance! Marcos Paulo de Souza (4): driver.h: Add macro VIR_DRV_CONN_CHECK_SERVER esx_driver: Use VIR_DRV_CONN_CHECK_SERVER macro hyperv_driver: Use VIR_DRV_CONN_CHECK_SERVER macro phyp_driver: Use VIR_DRV_CONN_CHECK_SERVER macro src/driver.h | 9 + src/esx/esx_driver.c | 7 +-- src/hyperv/hyperv_driver.c | 7 +-- src/phyp/phyp_driver.c | 6 +- 4 files changed, 12 insertions(+), 17 deletions(-) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH 4/4] phyp_driver: Use VIR_DRV_CONN_CHECK_SERVER macro
phyp needs a server to connect to, so use this macro in order to reuse
code.
Signed-off-by: Marcos Paulo de Souza
---
src/phyp/phyp_driver.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 67ce7903ba..beca1c6fc7 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -1141,11 +1141,7 @@ phypConnectOpen(virConnectPtr conn,
virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
-if (conn->uri->server == NULL) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("Missing server name in phyp:// URI"));
-return VIR_DRV_OPEN_ERROR;
-}
+VIR_DRV_CONN_CHECK_SERVER;
if (VIR_ALLOC(phyp_driver) < 0)
goto failure;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC PATCH 3/4] hyperv_driver: Use VIR_DRV_CONN_CHECK_SERVER macro
hyperv needs a server to connect to, so use this macro in order to reuse
code.
Signed-off-by: Marcos Paulo de Souza
---
src/hyperv/hyperv_driver.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 95496bdf73..a419f0ccee 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -128,12 +128,7 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr
auth,
virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
-/* Require server part */
-if (conn->uri->server == NULL) {
-virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("URI is missing the server part"));
-return VIR_DRV_OPEN_ERROR;
-}
+VIR_DRV_CONN_CHECK_SERVER;
/* Require auth */
if (auth == NULL || auth->cb == NULL) {
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH -v2 2/2] esx: Use ESX_VI_CHECK_ARG_LIST macro to avoid code duplication
By using this macro we can avoid boilerplate code to check for arrays of
objects from ESX driver. This replacement was done using the coccinelle
script bellow:
@@
identifier ptr;
@@
-if (!ptr || *ptr) { ... }
+ESX_VI_CHECK_ARG_LIST(ptr);
Signed-off-by: Marcos Paulo de Souza
---
Changes from v1:
* Use ESX_VI_CHECK_ARG_LIST macro from previous patch
* Use coccinelle script to check for all files inside esx directory
src/esx/esx_driver.c | 5 +-
src/esx/esx_network_driver.c | 10 +--
src/esx/esx_util.c | 5 +-
src/esx/esx_vi.c | 165 +++
src/esx/esx_vi_methods.c | 10 +--
src/esx/esx_vi_types.c | 51 +++
6 files changed, 49 insertions(+), 197 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 01bcc99962..06e1238385 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -626,10 +626,7 @@ esxConnectToHost(esxPrivate *priv,
? esxVI_ProductLine_ESX
: esxVI_ProductLine_GSX;
-if (!vCenterIPAddress || *vCenterIPAddress) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument"));
-return -1;
-}
+ESX_VI_CHECK_ARG_LIST(vCenterIPAddress);
if (esxUtil_ResolveHostname(conn->uri->server, ipAddress, NI_MAXHOST) < 0)
return -1;
diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
index b4f7f006d0..04118b4fa6 100644
--- a/src/esx/esx_network_driver.c
+++ b/src/esx/esx_network_driver.c
@@ -220,10 +220,7 @@ esxBandwidthToShapingPolicy(virNetDevBandwidthPtr
bandwidth,
{
int result = -1;
-if (!shapingPolicy || *shapingPolicy) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument"));
-return -1;
-}
+ESX_VI_CHECK_ARG_LIST(shapingPolicy);
if (!bandwidth->in || !bandwidth->out ||
bandwidth->in->average != bandwidth->out->average ||
@@ -589,10 +586,7 @@ static int
esxShapingPolicyToBandwidth(esxVI_HostNetworkTrafficShapingPolicy
*shapingPolicy,
virNetDevBandwidthPtr *bandwidth)
{
-if (!bandwidth || *bandwidth) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument"));
-return -1;
-}
+ESX_VI_CHECK_ARG_LIST(bandwidth);
if (!shapingPolicy || shapingPolicy->enabled != esxVI_Boolean_True)
return 0;
diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
index 2dd9f78569..d7210375fa 100644
--- a/src/esx/esx_util.c
+++ b/src/esx/esx_util.c
@@ -48,10 +48,7 @@ esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, virURIPtr
uri)
int autoAnswer;
char *tmp;
-if (!parsedUri || *parsedUri) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument"));
-return -1;
-}
+ESX_VI_CHECK_ARG_LIST(parsedUri);
if (VIR_ALLOC(*parsedUri) < 0)
return -1;
diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index 25fbdc7e44..d3c4f760ba 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -51,10 +51,7 @@ VIR_LOG_INIT("esx.esx_vi");
int \
esxVI_##_type##_Alloc(esxVI_##_type **ptrptr) \
{ \
-if (!ptrptr || *ptrptr) { \
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid
argument")); \
-return -1; \
-} \
+ESX_VI_CHECK_ARG_LIST(ptrptr); \
\
if (VIR_ALLOC(*ptrptr) < 0) \
return -1; \
@@ -372,10 +369,7 @@ esxVI_CURL_Download(esxVI_CURL *curl, const char *url,
char **content,
virBuffer buffer = VIR_BUFFER_INITIALIZER;
int responseCode = 0;
-if (!content || *content) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument"));
-return -1;
-}
+ESX_VI_CHECK_ARG_LIST(content);
if (length && *length > 0) {
/*
@@ -1762,10 +1756,7 @@ esxVI_List_DeepCopy(esxVI_List **destList, esxVI_List
*srcList,
esxVI_List *dest = NULL;
esxVI_List *src = NULL;
-if (!destList || *destList) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument"));
-return -1;
-}
+ESX_VI_CHECK_ARG_LIST(destList);
for (src = srcList; src; src = src->_next) {
if (deepCopyFunc(, src) < 0 ||
@@ -2170,10 +2161,7 @@ esxVI_LookupObjectContentByType(esxVI_Context *ctx,
bool propertySpec_isAppended = false;
esxVI_PropertyFilterSpec *propertyFilterSpec = NULL;
-if (!objectContentList || *objectContentList) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument"));
-return -1;
-}
+ESX_VI_CHECK_ARG_LIST(objectContentList);
if (esxVI_ObjectSpec_Alloc() < 0)
return -1;
@@ -2372,10 +2360,7 @@ esxVI_GetVirtualMachineQuestionInfo
{
esxVI_DynamicProperty *dynamicPro
[libvirt] [PATCH -v2 1/2] esx_util.h: Add ESX_VI_CHECK_ARG_LIST macro
This macro avoids code duplication when checking for arrays of objects.
Signed-off-by: Marcos Paulo de Souza
---
Changes from v1:
* Change VIR_ERR_INVALID_ARG to VIR_ERR_INTERNAL_ERROR (Michal)
* Change esxVI_checkArgList to ESX_VI_CHECK_ARG_LIST (Matthias)
src/esx/esx_util.h | 8
1 file changed, 8 insertions(+)
diff --git a/src/esx/esx_util.h b/src/esx/esx_util.h
index c6f14bb2d9..63602bf3cb 100644
--- a/src/esx/esx_util.h
+++ b/src/esx/esx_util.h
@@ -26,6 +26,14 @@
# include "internal.h"
# include "viruri.h"
+#define ESX_VI_CHECK_ARG_LIST(val) \
+do { \
+if (!val || *val) { \
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid
argument")); \
+return -1; \
+} \
+} while (0)
+
typedef struct _esxUtil_ParsedUri esxUtil_ParsedUri;
struct _esxUtil_ParsedUri {
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH -v2 0/2] esx: use coccinelle to avoid code duplication
Hi guys, in this second version I address the comments of Michal and Matthias. Changes from v1 are placed in each patch. Please let me know if you have more suggestions for the approach taken. Thanks, Marcos Paulo de Souza (2): esx_util.h: Add ESX_VI_CHECK_ARG_LIST macro esx: Use ESX_VI_CHECK_ARG_LIST macro to avoid code duplication src/esx/esx_driver.c | 5 +- src/esx/esx_network_driver.c | 10 +-- src/esx/esx_util.c | 5 +- src/esx/esx_util.h | 8 ++ src/esx/esx_vi.c | 165 +++ src/esx/esx_vi_methods.c | 10 +-- src/esx/esx_vi_types.c | 51 +++ 7 files changed, 57 insertions(+), 197 deletions(-) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] esx: Add esxVI_checkArgList macro
On Tue, Jul 03, 2018 at 11:31:36AM +0200, Michal Prívozník wrote:
> On 07/03/2018 04:20 AM, Marcos Paulo de Souza wrote:
> > This macro avoids code duplication when checking for arrays of objects.
> >
> > Signed-off-by: Marcos Paulo de Souza
> > ---
> > src/esx/esx_vi.c | 189 ---
> > 1 file changed, 46 insertions(+), 143 deletions(-)
> >
> > diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
> > index 25fbdc7e44..212300dbff 100644
> > --- a/src/esx/esx_vi.c
> > +++ b/src/esx/esx_vi.c
> > @@ -41,6 +41,16 @@
> >
> > VIR_LOG_INIT("esx.esx_vi");
> >
> > +#define esxVI_checkArgList(val) \
> > +do { \
> > +if (!val || *val) { \
> > +virReportError(VIR_ERR_INVALID_ARG, "%s", _("Invalid
> > argument")); \
>
> Actually, this is one of the few places where VIR_ERR_INTERNAL_ERROR is
> not misused and makes sense. The only way how this error can be reported
> is if there's a bug in our code, not because of some input user made.
>
> There are also other occurrences of this pattern. coccinelle helped to
> find other. I used the following spatch:
>
> @@
> identifier ptr;
> @@
>
> - (!ptr || *ptr)
> + (esxVI_checkArgList(ptr))
>
>
> Mind putting them here too? Otherwise the patch looks good.
No problem. I will take a look into coccinele and resend the patch once all
other places are covered. Thanks for pushing the other patches and fixing the
code style.
>
> Michal
--
Thanks,
Marcos
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] esx_driver: Use virCheckFlag macro
Instead of duplicating code to do the same checking. Now all functions
of virHypervisorDriver from esx driver are using this macro.
Signed-off-by: Marcos Paulo de Souza
---
src/esx/esx_driver.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 60aa5fc252..705b0d1a41 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -2488,10 +2488,7 @@ esxDomainSetVcpusFlags(virDomainPtr domain, unsigned int
nvcpus,
esxVI_TaskInfoState taskInfoState;
char *taskInfoErrorMessage = NULL;
-if (flags != VIR_DOMAIN_AFFECT_LIVE) {
-virReportError(VIR_ERR_INVALID_ARG, _("unsupported flags: (0x%x)"),
flags);
-return -1;
-}
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE, -1);
if (nvcpus < 1) {
virReportError(VIR_ERR_INVALID_ARG, "%s",
@@ -2570,10 +2567,7 @@ esxDomainGetVcpusFlags(virDomainPtr domain, unsigned int
flags)
esxVI_ObjectContent *hostSystem = NULL;
esxVI_DynamicProperty *dynamicProperty = NULL;
-if (flags != (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_VCPU_MAXIMUM)) {
-virReportError(VIR_ERR_INVALID_ARG, _("unsupported flags: (0x%x)"),
flags);
-return -1;
-}
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_VCPU_MAXIMUM, -1);
if (priv->maxVcpus > 0)
return priv->maxVcpus;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] esx_vi.c: Simplify error handling in MachineByName
The same pattern is used in lots of other places.
Signed-off-by: Marcos Paulo de Souza
---
src/esx/esx_vi.c | 14 --
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index 43ff7ea048..25fbdc7e44 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -3014,16 +3014,10 @@ esxVI_LookupVirtualMachineByName(esxVI_Context *ctx,
const char *name,
break;
}
-if (!(*virtualMachine)) {
-if (occurrence == esxVI_Occurrence_OptionalItem) {
-result = 0;
-
-goto cleanup;
-} else {
-virReportError(VIR_ERR_NO_DOMAIN,
- _("Could not find domain with name '%s'"), name);
-goto cleanup;
-}
+if (!(*virtualMachine) && occurrence != esxVI_Occurrence_OptionalItem) {
+virReportError(VIR_ERR_NO_DOMAIN,
+ _("Could not find domain with name '%s'"), name);
+goto cleanup;
}
result = 0;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] esx: Add esxVI_checkArgList macro
This macro avoids code duplication when checking for arrays of objects.
Signed-off-by: Marcos Paulo de Souza
---
src/esx/esx_vi.c | 189 ---
1 file changed, 46 insertions(+), 143 deletions(-)
diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index 25fbdc7e44..212300dbff 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -41,6 +41,16 @@
VIR_LOG_INIT("esx.esx_vi");
+#define esxVI_checkArgList(val) \
+do { \
+if (!val || *val) { \
+virReportError(VIR_ERR_INVALID_ARG, "%s", _("Invalid argument")); \
+return -1; \
+} \
+} while (0)
+
+
+
#define ESX_VI__SOAP__RESPONSE_XPATH(_type) \
((char *)"/soapenv:Envelope/soapenv:Body/" \
"vim:"_type"Response/vim:returnval")
@@ -51,11 +61,7 @@ VIR_LOG_INIT("esx.esx_vi");
int \
esxVI_##_type##_Alloc(esxVI_##_type **ptrptr) \
{ \
-if (!ptrptr || *ptrptr) { \
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid
argument")); \
-return -1; \
-} \
- \
+esxVI_checkArgList(ptrptr); \
if (VIR_ALLOC(*ptrptr) < 0) \
return -1; \
return 0; \
@@ -372,10 +378,7 @@ esxVI_CURL_Download(esxVI_CURL *curl, const char *url,
char **content,
virBuffer buffer = VIR_BUFFER_INITIALIZER;
int responseCode = 0;
-if (!content || *content) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument"));
-return -1;
-}
+esxVI_checkArgList(content);
if (length && *length > 0) {
/*
@@ -1762,10 +1765,7 @@ esxVI_List_DeepCopy(esxVI_List **destList, esxVI_List
*srcList,
esxVI_List *dest = NULL;
esxVI_List *src = NULL;
-if (!destList || *destList) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument"));
-return -1;
-}
+esxVI_checkArgList(destList);
for (src = srcList; src; src = src->_next) {
if (deepCopyFunc(, src) < 0 ||
@@ -2170,10 +2170,7 @@ esxVI_LookupObjectContentByType(esxVI_Context *ctx,
bool propertySpec_isAppended = false;
esxVI_PropertyFilterSpec *propertyFilterSpec = NULL;
-if (!objectContentList || *objectContentList) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument"));
-return -1;
-}
+esxVI_checkArgList(objectContentList);
if (esxVI_ObjectSpec_Alloc() < 0)
return -1;
@@ -2372,10 +2369,7 @@ esxVI_GetVirtualMachineQuestionInfo
{
esxVI_DynamicProperty *dynamicProperty;
-if (!questionInfo || *questionInfo) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument"));
-return -1;
-}
+esxVI_checkArgList(questionInfo);
for (dynamicProperty = virtualMachine->propSet; dynamicProperty;
dynamicProperty = dynamicProperty->_next) {
@@ -2447,10 +2441,7 @@ esxVI_GetInt(esxVI_ObjectContent *objectContent, const
char *propertyName,
{
esxVI_DynamicProperty *dynamicProperty;
-if (!value || *value) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument"));
-return -1;
-}
+esxVI_checkArgList(value);
for (dynamicProperty = objectContent->propSet; dynamicProperty;
dynamicProperty = dynamicProperty->_next) {
@@ -2479,10 +2470,7 @@ esxVI_GetLong(esxVI_ObjectContent *objectContent, const
char *propertyName,
{
esxVI_DynamicProperty *dynamicProperty;
-if (!value || *value) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument"));
-return -1;
-}
+esxVI_checkArgList(value);
for (dynamicProperty = objectContent->propSet; dynamicProperty;
dynamicProperty = dynamicProperty->_next) {
@@ -2512,10 +2500,7 @@ esxVI_GetStringValue(esxVI_ObjectContent *objectContent,
{
esxVI_DynamicProperty *dynamicProperty;
-if (!value || *value) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument"));
-return -1;
-}
+esxVI_checkArgList(value);
for (dynamicProperty = objectContent->propSet; dynamicProperty;
dynamicProperty = dynamicProperty->_next) {
@@ -2549,10 +2534,7 @@ esxVI_GetManagedObjectReference(esxVI_ObjectContent
*objectContent,
{
esxVI_DynamicProperty *dynamicProperty;
-if (!value || *value) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument"));
-return -1;
-}
+esxVI_checkArgList(value);
for (dynamicProperty = objectContent->propSet; dynamicProperty;
dynamicProperty = dynamicProperty->_next) {
@@ -2863,10 +2845,7 @@ esxVI_GetSnapshotTreeBySnapshot
{
esxVI_VirtualMachineSn
[libvirt] [PATCH] esx_driver: Simplify IsEncrypted and IsSecure
Signed-off-by: Marcos Paulo de Souza
---
src/esx/esx_driver.c | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 60aa5fc252..bb8eeecd42 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -4000,11 +4000,7 @@ esxConnectIsEncrypted(virConnectPtr conn)
{
esxPrivate *priv = conn->privateData;
-if (STRCASEEQ(priv->parsedUri->transport, "https")) {
-return 1;
-} else {
-return 0;
-}
+return STRCASEEQ(priv->parsedUri->transport, "https");
}
@@ -4014,11 +4010,7 @@ esxConnectIsSecure(virConnectPtr conn)
{
esxPrivate *priv = conn->privateData;
-if (STRCASEEQ(priv->parsedUri->transport, "https")) {
-return 1;
-} else {
-return 0;
-}
+return STRCASEEQ(priv->parsedUri->transport, "https");
}
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] appdev-guide/Connections: Remove duplicated entries
On Mon, Jun 25, 2018 at 05:13:26PM +0200, Erik Skultety wrote: > On Fri, Jun 22, 2018 at 05:44:18PM -0300, Marcos Paulo de Souza wrote: > > On Mon, Jun 25, 2018 at 01:35:33PM +0200, Erik Skultety wrote: > > > On Sat, Jun 23, 2018 at 11:32:55AM -0300, Marcos Paulo de Souza wrote: > > > > Both /hosts/cpu/arch and /hosts/cpu/features appear twice in the > > > > documentation. > > > > > > > > Signed-off-by: Marcos Paulo de Souza > > > > --- > > > > > > > > This is my first submission to libvirt, so let me know if I forgot > > > > something. > > > > > > Which repo is this patch against? > > > > This patch is againt the appdev-guide located here: > > https://libvirt.org/git/?p=libvirt-appdev-guide.git;a=summary > > > > The repository seems to be the source of the documenation located in > > libvirt.org: > > https://libvirt.org/devguide.html > > > > Is there other place to send patches for appdev-guide? > > No, but usually we change patch subject prefix to PATCH-project so that it's > obvious which project you're sending the patch against, because from the > commit > subject itself it's not apparent at first sight, especially when noone has > touched that git for over 6 years. The document itself was never even finished > and regarding the publican we basically it by moving the static content to the > main repo. Anyhow, I'm fairly sceptical anyone ever used the development guide > solely because of how much it's missing, so I don't know how much sense it > makes > to push the patch and whether we keep building the repo so that the change > would actually be propagated, even though it's correct, but Dan will know the > answer for sure, so I'm CCing him. OK, fair enough. As I saw a commit in the last months, I tought it would be good to improve it. But OK, I will try to read the docs from the main repo, thanks for pointing it out. Maybe it would be nice to state this in the libvirt.org. Thanks, > > Erik -- Thanks, Marcos -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] appdev-guide/Connections: Remove duplicated entries
On Mon, Jun 25, 2018 at 01:35:33PM +0200, Erik Skultety wrote: > On Sat, Jun 23, 2018 at 11:32:55AM -0300, Marcos Paulo de Souza wrote: > > Both /hosts/cpu/arch and /hosts/cpu/features appear twice in the > > documentation. > > > > Signed-off-by: Marcos Paulo de Souza > > --- > > > > This is my first submission to libvirt, so let me know if I forgot > > something. > > Which repo is this patch against? This patch is againt the appdev-guide located here: https://libvirt.org/git/?p=libvirt-appdev-guide.git;a=summary The repository seems to be the source of the documenation located in libvirt.org: https://libvirt.org/devguide.html Is there other place to send patches for appdev-guide? > > Erik -- Thanks, Marcos -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] appdev-guide/Connections: Remove duplicated entries
Both /hosts/cpu/arch and /hosts/cpu/features appear twice in the documentation. Signed-off-by: Marcos Paulo de Souza --- This is my first submission to libvirt, so let me know if I forgot something. en-US/Connections.xml | 11 --- 1 file changed, 11 deletions(-) diff --git a/en-US/Connections.xml b/en-US/Connections.xml index 96fe011..d7b6af6 100644 --- a/en-US/Connections.xml +++ b/en-US/Connections.xml @@ -963,17 +963,6 @@ int main(int argc, char *argv[]) CPU architecture. As of this writing, all libvirt drivers initialize this from the output of uname(2). - - /host/cpu/features is an optional sub-document that describes additional cpu - features present on the host. As of this writing, it is only used by the - xen driver to report on the presence or lack of the svm or vmx flag, and to - report on the presence or lack of the pae flag. - - - /host/cpu/arch is a required XML node that describes the underlying - host CPU architecture. As of this writing, all libvirt drivers - initialize this from the output of uname(2). - /host/cpu/model is an optional element that describes the CPU model that the host CPUs most closely resemble. The list of CPU models -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
