[libvirt PATCH 6/6] libxl: remove cleanup label from libxlDomainMigrationSrcPerform
Use VIR_AUTOCLOSE for the remaining file descriptor that uses
manual cleanup and remove the label.
Signed-off-by: Ján Tomko
---
src/libxl/libxl_migration.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index c5ec80139f..800a6b0365 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -1183,13 +1183,13 @@ libxlDomainMigrationSrcPerform(libxlDriverPrivate
*driver,
char portstr[100];
g_autoptr(virURI) uri = NULL;
virNetSocket *sock;
-int sockfd = -1;
+VIR_AUTOCLOSE sockfd = -1;
int ret = -1;
/* parse dst host:port from uri */
uri = virURIParse(uri_str);
if (uri == NULL || uri->server == NULL || uri->port == 0)
-goto cleanup;
+return -1;
hostname = uri->server;
port = uri->port;
@@ -1199,11 +1199,11 @@ libxlDomainMigrationSrcPerform(libxlDriverPrivate
*driver,
if (virNetSocketNewConnectTCP(hostname, portstr,
AF_UNSPEC,
) < 0)
-goto cleanup;
+return -1;
if (virNetSocketSetBlocking(sock, true) < 0) {
virObjectUnref(sock);
-goto cleanup;
+return -1;
}
sockfd = virNetSocketDupFD(sock, true);
@@ -1229,8 +1229,6 @@ libxlDomainMigrationSrcPerform(libxlDriverPrivate *driver,
libxlDomainObjEndJob(driver, vm);
}
- cleanup:
-VIR_FORCE_CLOSE(sockfd);
return ret;
}
--
2.34.1
[libvirt PATCH 1/6] Use g_auto for virURI almost everywhere
Convert all the cases where we can unconditionally free
the virURI at the end of scope.
In libxlDomainMigrationDstPrepare, uri is only filled
if uri_in was present, so moving the virURIFree out of
the condition is safe.
Signed-off-by: Ján Tomko
---
src/esx/esx_driver.c| 3 +--
src/libxl/libxl_migration.c | 7 ++-
src/qemu/qemu_migration.c | 3 +--
src/vmx/vmx.c | 3 +--
src/vz/vz_driver.c | 3 +--
src/vz/vz_sdk.c | 3 +--
tests/viruritest.c | 3 +--
7 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 79b991f36f..9dc5489411 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -3658,7 +3658,7 @@ esxDomainMigratePerform(virDomainPtr domain,
{
int result = -1;
esxPrivate *priv = domain->conn->privateData;
-virURI *parsedUri = NULL;
+g_autoptr(virURI) parsedUri = NULL;
char *saveptr;
char *path_resourcePool;
char *path_hostSystem;
@@ -3779,7 +3779,6 @@ esxDomainMigratePerform(virDomainPtr domain,
result = 0;
cleanup:
-virURIFree(parsedUri);
esxVI_ObjectContent_Free();
esxVI_Event_Free();
esxVI_ManagedObjectReference_Free();
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 0fd2b3206e..c5ec80139f 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -637,7 +637,7 @@ libxlDomainMigrationDstPrepare(virConnectPtr dconn,
char *xmlout = NULL;
unsigned short port;
char portstr[100];
-virURI *uri = NULL;
+g_autoptr(virURI) uri = NULL;
virNetSocket **socks = NULL;
size_t nsocks = 0;
int nsocks_listen = 0;
@@ -795,8 +795,6 @@ libxlDomainMigrationDstPrepare(virConnectPtr dconn,
libxlMigrationCookieFree(mig);
if (!uri_in)
VIR_FREE(hostname);
-else
-virURIFree(uri);
virObjectUnref(args);
virDomainObjEndAPI();
virObjectUnref(cfg);
@@ -1183,7 +1181,7 @@ libxlDomainMigrationSrcPerform(libxlDriverPrivate *driver,
char *hostname = NULL;
unsigned short port = 0;
char portstr[100];
-virURI *uri = NULL;
+g_autoptr(virURI) uri = NULL;
virNetSocket *sock;
int sockfd = -1;
int ret = -1;
@@ -1233,7 +1231,6 @@ libxlDomainMigrationSrcPerform(libxlDriverPrivate *driver,
cleanup:
VIR_FORCE_CLOSE(sockfd);
-virURIFree(uri);
return ret;
}
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 6e5b3839fb..3d7d5efda3 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2107,7 +2107,7 @@ qemuMigrationSrcGraphicsRelocate(virQEMUDriver *driver,
int ret = -1;
const char *listenAddress = NULL;
virSocketAddr addr;
-virURI *uri = NULL;
+g_autoptr(virURI) uri = NULL;
int type = -1;
int port = -1;
int tlsPort = -1;
@@ -2192,7 +2192,6 @@ qemuMigrationSrcGraphicsRelocate(virQEMUDriver *driver,
}
cleanup:
-virURIFree(uri);
return ret;
}
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index e24e9211aa..282e9562e0 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -2903,7 +2903,7 @@ virVMXParseSerial(virVMXContext *ctx, virConf *conf, int
port,
char network_endPoint_name[48] = "";
char *network_endPoint = NULL;
-virURI *parsedUri = NULL;
+g_autoptr(virURI) parsedUri = NULL;
if (def == NULL || *def != NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument"));
@@ -3051,7 +3051,6 @@ virVMXParseSerial(virVMXContext *ctx, virConf *conf, int
port,
VIR_FREE(fileType);
VIR_FREE(fileName);
VIR_FREE(network_endPoint);
-virURIFree(parsedUri);
return result;
}
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 82323cb1b2..017c084ede 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -3074,7 +3074,7 @@ vzDomainMigratePerformStep(virDomainObj *dom,
{
int ret = -1;
struct vzDomObj *privdom = dom->privateData;
-virURI *vzuri = NULL;
+g_autoptr(virURI) vzuri = NULL;
const char *miguri = NULL;
const char *dname = NULL;
vzMigrationCookie *mig = NULL;
@@ -3117,7 +3117,6 @@ vzDomainMigratePerformStep(virDomainObj *dom,
cleanup:
if (job)
vzDomainObjEndJob(dom);
-virURIFree(vzuri);
vzMigrationCookieFree(mig);
return ret;
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 43d528820e..5ea3f29a8f 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1133,7 +1133,7 @@ prlsdkGetSerialInfo(PRL_HANDLE serialPort,
virDomainChrDef *chr)
char *friendlyName = NULL;
PRL_SERIAL_PORT_SOCKET_OPERATION_MODE socket_mode;
char *uristr = NULL;
-virURI *uri = NULL;
+g_autoptr(virURI) uri = NULL;
int ret = -1;
chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL;
@@ -1196,7 +1196,6 @@ prlsdkGetSerialInfo(PRL_HANDLE serialPort,
virDomainChrDef *chr)
cleanup:
VIR_FREE(friendlyName);
VIR_FREE(uristr);
-
[libvirt PATCH 5/6] qemu: remove cleanup label from qemuMigrationSrcGraphicsRelocate
Remove the label and use 'rc' instead of 'ret'.
Signed-off-by: Ján Tomko
---
src/qemu/qemu_migration.c | 27 +++
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 3d7d5efda3..0ed0066e27 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2104,7 +2104,6 @@ qemuMigrationSrcGraphicsRelocate(virQEMUDriver *driver,
const char *graphicsuri)
{
qemuDomainObjPrivate *priv = vm->privateData;
-int ret = -1;
const char *listenAddress = NULL;
virSocketAddr addr;
g_autoptr(virURI) uri = NULL;
@@ -2112,12 +2111,13 @@ qemuMigrationSrcGraphicsRelocate(virQEMUDriver *driver,
int port = -1;
int tlsPort = -1;
const char *tlsSubject = NULL;
+int rc = -1;
if (!cookie || (!cookie->graphics && !graphicsuri))
return 0;
if (graphicsuri && !(uri = virURIParse(graphicsuri)))
-goto cleanup;
+return -1;
if (cookie->graphics) {
type = cookie->graphics->type;
@@ -2140,7 +2140,7 @@ qemuMigrationSrcGraphicsRelocate(virQEMUDriver *driver,
if ((type = virDomainGraphicsTypeFromString(uri->scheme)) < 0) {
virReportError(VIR_ERR_INVALID_ARG,
_("unknown graphics type %s"), uri->scheme);
-goto cleanup;
+return -1;
}
if (uri->server)
@@ -2156,7 +2156,7 @@ qemuMigrationSrcGraphicsRelocate(virQEMUDriver *driver,
virReportError(VIR_ERR_INVALID_ARG,
_("invalid tlsPort number: %s"),
param->value);
-goto cleanup;
+return -1;
}
} else if (STRCASEEQ(param->name, "tlsSubject")) {
tlsSubject = param->value;
@@ -2167,32 +2167,27 @@ qemuMigrationSrcGraphicsRelocate(virQEMUDriver *driver,
/* QEMU doesn't support VNC relocation yet, so
* skip it to avoid generating an error
*/
-if (type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
-ret = 0;
-goto cleanup;
-}
+if (type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE)
+return 0;
/* Older libvirt sends port == 0 for listen type='none' graphics. It's
* safe to ignore such requests since relocation to unknown port does
* not make sense in general.
*/
-if (port <= 0 && tlsPort <= 0) {
-ret = 0;
-goto cleanup;
-}
+if (port <= 0 && tlsPort <= 0)
+return 0;
if (qemuDomainObjEnterMonitorAsync(driver, vm,
VIR_ASYNC_JOB_MIGRATION_OUT) == 0) {
qemuDomainJobPrivate *jobPriv = priv->job.privateData;
-ret = qemuMonitorGraphicsRelocate(priv->mon, type, listenAddress,
+rc = qemuMonitorGraphicsRelocate(priv->mon, type, listenAddress,
port, tlsPort, tlsSubject);
-jobPriv->spiceMigration = !ret;
+jobPriv->spiceMigration = !rc;
qemuDomainObjExitMonitor(vm);
}
- cleanup:
-return ret;
+return rc;
}
--
2.34.1
[libvirt PATCH 4/6] vmx: use g_autofree in virVMXParseSerial
Signed-off-by: Ján Tomko --- src/vmx/vmx.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 282e9562e0..43ddee5bb6 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2895,13 +2895,13 @@ virVMXParseSerial(virVMXContext *ctx, virConf *conf, int port, bool startConnected = false; char fileType_name[48] = ""; -char *fileType = NULL; +g_autofree char *fileType = NULL; char fileName_name[48] = ""; -char *fileName = NULL; +g_autofree char *fileName = NULL; char network_endPoint_name[48] = ""; -char *network_endPoint = NULL; +g_autofree char *network_endPoint = NULL; g_autoptr(virURI) parsedUri = NULL; @@ -3048,10 +3048,6 @@ virVMXParseSerial(virVMXContext *ctx, virConf *conf, int port, g_clear_pointer(def, virDomainChrDefFree); } -VIR_FREE(fileType); -VIR_FREE(fileName); -VIR_FREE(network_endPoint); - return result; } -- 2.34.1
[libvirt PATCH 0/6] Use virURIFree less (glib chronicles)
Ján Tomko (6): Use g_auto for virURI almost everywhere tests: remove pointless label in testURIParse vz: refactor testURIParse vmx: use g_autofree in virVMXParseSerial qemu: remove cleanup label from qemuMigrationSrcGraphicsRelocate libxl: remove cleanup label from libxlDomainMigrationSrcPerform src/esx/esx_driver.c| 3 +-- src/libxl/libxl_migration.c | 17 ++--- src/qemu/qemu_migration.c | 30 -- src/vmx/vmx.c | 13 - src/vz/vz_driver.c | 3 +-- src/vz/vz_sdk.c | 31 +++ tests/viruritest.c | 14 +- 7 files changed, 40 insertions(+), 71 deletions(-) -- 2.34.1
[libvirt PATCH 2/6] tests: remove pointless label in testURIParse
Signed-off-by: Ján Tomko
---
tests/viruritest.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/tests/viruritest.c b/tests/viruritest.c
index 7eb7c9cb67..cd6ce57371 100644
--- a/tests/viruritest.c
+++ b/tests/viruritest.c
@@ -45,7 +45,6 @@ struct URIParseData {
static int testURIParse(const void *args)
{
-int ret = -1;
g_autoptr(virURI) uri = NULL;
const struct URIParseData *data = args;
g_autofree char *uristr = NULL;
@@ -53,7 +52,7 @@ static int testURIParse(const void *args)
bool fail = false;
if (!(uri = virURIParse(data->uri)))
-goto cleanup;
+return -1;
if (STRNEQ(uri->scheme, data->scheme)) {
VIR_TEST_DEBUG("Expected scheme '%s', actual '%s'",
@@ -120,7 +119,7 @@ static int testURIParse(const void *args)
uri->query = virURIFormatParams(uri);
if (!(uristr = virURIFormat(uri)))
-goto cleanup;
+return -1;
if (STRNEQ(uristr, data->uri_out)) {
VIR_TEST_DEBUG("URI did not roundtrip, expect '%s', actual '%s'",
@@ -129,11 +128,9 @@ static int testURIParse(const void *args)
}
if (fail)
-goto cleanup;
+return -1;
-ret = 0;
- cleanup:
-return ret;
+return 0;
}
--
2.34.1
[libvirt PATCH 3/6] vz: refactor testURIParse
Use g_autofree for the two strings still using manual cleanup
and remove the pointless cleanup label.
Signed-off-by: Ján Tomko
---
src/vz/vz_sdk.c | 28 ++--
1 file changed, 10 insertions(+), 18 deletions(-)
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 5ea3f29a8f..ecf610d7db 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1130,26 +1130,25 @@ prlsdkGetSerialInfo(PRL_HANDLE serialPort,
virDomainChrDef *chr)
PRL_RESULT pret;
PRL_UINT32 serialPortIndex;
PRL_UINT32 emulatedType;
-char *friendlyName = NULL;
+g_autofree char *friendlyName = NULL;
PRL_SERIAL_PORT_SOCKET_OPERATION_MODE socket_mode;
-char *uristr = NULL;
+g_autofree char *uristr = NULL;
g_autoptr(virURI) uri = NULL;
-int ret = -1;
chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL;
pret = PrlVmDev_GetIndex(serialPort, );
-prlsdkCheckRetGoto(pret, cleanup);
+prlsdkCheckRetExit(pret, -1);
chr->target.port = serialPortIndex;
pret = PrlVmDev_GetEmulatedType(serialPort, );
-prlsdkCheckRetGoto(pret, cleanup);
+prlsdkCheckRetExit(pret, -1);
if (!(friendlyName = prlsdkGetStringParamVar(PrlVmDev_GetFriendlyName,
serialPort)))
-goto cleanup;
+return -1;
pret = PrlVmDevSerial_GetSocketMode(serialPort, _mode);
-prlsdkCheckRetGoto(pret, cleanup);
+prlsdkCheckRetExit(pret, -1);
switch (emulatedType) {
case PDT_USE_OUTPUT_FILE:
@@ -1169,7 +1168,7 @@ prlsdkGetSerialInfo(PRL_HANDLE serialPort,
virDomainChrDef *chr)
chr->source->type = VIR_DOMAIN_CHR_TYPE_TCP;
uristr = g_strdup_printf("tcp://%s", friendlyName);
if (!(uri = virURIParse(uristr)))
-goto cleanup;
+return -1;
chr->source->data.tcp.host = g_strdup(uri->server);
chr->source->data.tcp.service = g_strdup_printf("%d", uri->port);
chr->source->data.tcp.listen = socket_mode == PSP_SERIAL_SOCKET_SERVER;
@@ -1178,7 +1177,7 @@ prlsdkGetSerialInfo(PRL_HANDLE serialPort,
virDomainChrDef *chr)
chr->source->type = VIR_DOMAIN_CHR_TYPE_UDP;
uristr = g_strdup_printf("udp://%s", friendlyName);
if (!(uri = virURIParse(uristr)))
-goto cleanup;
+return -1;
chr->source->data.udp.bindHost = g_strdup(uri->server);
chr->source->data.udp.bindService = g_strdup_printf("%d", uri->port);
chr->source->data.udp.connectHost = g_strdup(uri->server);
@@ -1187,17 +1186,10 @@ prlsdkGetSerialInfo(PRL_HANDLE serialPort,
virDomainChrDef *chr)
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unknown serial type: %X"), emulatedType);
-goto cleanup;
-break;
+return -1;
}
-ret = 0;
-
- cleanup:
-VIR_FREE(friendlyName);
-VIR_FREE(uristr);
-
-return ret;
+return 0;
}
--
2.34.1
Re: [PATCH] Add support for network backed NVRAM
On Tue, Jun 21, 2022 at 5:53 PM Rohit Kumar wrote: > > Signed-off-by: Rohit Kumar Reviewed-by: Ani Sinha > --- > NEWS.rst | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/NEWS.rst b/NEWS.rst > index 9a8624a97e..9766dbe1df 100644 > --- a/NEWS.rst > +++ b/NEWS.rst > @@ -17,6 +17,11 @@ v8.5.0 (unreleased) > > * **New features** > > + * qemu: Introduce support for network backed NVRAM > + > +Users can now use remote store NVRAM image by specifying attribute I would say 'newly introduced attribute' so that it is clear what already exists and what was newly introduced. > +`type='network'` with `` element. > + >* qemu: Add support for post-copy migration recovery > > A new ``VIR_MIGRATE_POSTCOPY_RESUME`` flag (``virsh migrate > --postcopy-resume``) > -- > 2.25.1 >
Re: [PATCH] libxl: Fix domain startup failure error reporting
On 6/21/22 3:55 AM, Michal Prívozník wrote:
> On 6/17/22 23:29, Cole Robinson wrote:
>> When domain startup fails, domain cleanup calls
>> libxlNetworkUnwindDevices, which calls virGetConnectNetwork, which
>> is a top level API entry point, which resets the initial saved error,
>> leading to clients seeing:
>>
>> error: An error occurred, but the cause is unknown
>>
>> This preserves the error from before virGetConnectNetwork is called.
>>
>> Signed-off-by: Cole Robinson
>> ---
>> src/libxl/libxl_domain.c | 7 ++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
>> index 17b347de4e..bda110e9e6 100644
>> --- a/src/libxl/libxl_domain.c
>> +++ b/src/libxl/libxl_domain.c
>> @@ -830,12 +830,17 @@ libxlNetworkUnwindDevices(virDomainDef *def)
>> /* cleanup actual device */
>> virDomainNetRemoveHostdev(def, net);
>> if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
>> -g_autoptr(virConnect) conn = virGetConnectNetwork();
>> +g_autoptr(virConnect) conn = NULL;
>> +virErrorPtr save_err;
>> +
>> +virErrorPreserveLast(_err);
>> +conn = virGetConnectNetwork();
>>
>> if (conn)
>> virDomainNetReleaseActualDevice(conn, def, net);
>> else
>> VIR_WARN("Unable to release network device '%s'",
>> NULLSTR(net->ifname));
>> +virErrorRestore(_err);
>> }
>> }
>> }
>
> This fixes this particular function. I wonder whether we should mimic
> what QEMU driver does and wrap whole qemuProcessShutdown(), I mean
> libxlDomainCleanup() in virErrorPreserveLast(). Something like this:
>
> diff --git i/src/libxl/libxl_domain.c w/src/libxl/libxl_domain.c
> index bda110e9e6..8e8ddd284a 100644
> --- i/src/libxl/libxl_domain.c
> +++ w/src/libxl/libxl_domain.c
> @@ -908,10 +908,13 @@ libxlDomainCleanup(libxlDriverPrivate *driver,
> virHostdevManager *hostdev_mgr = driver->hostdevMgr;
> unsigned int hostdev_flags = VIR_HOSTDEV_SP_PCI;
> size_t i;
> +virErrorPtr save_err;
>
> VIR_DEBUG("Cleaning up domain with id '%d' and name '%s'",
>vm->def->id, vm->def->name);
>
> +virErrorPreserveLast(_err);
> +
> hostdev_flags |= VIR_HOSTDEV_SP_USB;
>
> /* Call hook with stopped operation. Ignore error and continue with
> cleanup */
> @@ -984,6 +987,7 @@ libxlDomainCleanup(libxlDriverPrivate *driver,
> VIR_HOOK_SUBOP_END, NULL));
>
> virDomainObjRemoveTransientDef(vm);
> +virErrorRestore(_err);
> }
>
> /*
> @@ -1245,6 +1249,7 @@ libxlDomainStartPrepare(libxlDriverPrivate *driver,
> {
> virHostdevManager *hostdev_mgr = driver->hostdevMgr;
> unsigned int hostdev_flags = VIR_HOSTDEV_SP_PCI | VIR_HOSTDEV_SP_USB;
> +virErrorPtr save_err;
>
> if (virDomainObjSetDefTransient(driver->xmlopt, vm, NULL) < 0)
> return -1;
> @@ -1272,10 +1277,12 @@ libxlDomainStartPrepare(libxlDriverPrivate *driver,
> return 0;
>
> error:
> +virErrorPreserveLast(_err);
> libxlNetworkUnwindDevices(vm->def);
> virHostdevReAttachDomainDevices(hostdev_mgr, LIBXL_DRIVER_INTERNAL_NAME,
> vm->def, hostdev_flags);
> virDomainObjRemoveTransientDef(vm);
> +virErrorRestore(_err);
> return -1;
> }
>
>
> If this works, replace your patch with this diff, apply my:
>
> Reviewed-by: Michal Privoznik
Thanks, I made that change and pushed now
- Cole
[PATCH] Add support for network backed NVRAM
Signed-off-by: Rohit Kumar --- NEWS.rst | 5 + 1 file changed, 5 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 9a8624a97e..9766dbe1df 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -17,6 +17,11 @@ v8.5.0 (unreleased) * **New features** + * qemu: Introduce support for network backed NVRAM + +Users can now use remote store NVRAM image by specifying attribute +`type='network'` with `` element. + * qemu: Add support for post-copy migration recovery A new ``VIR_MIGRATE_POSTCOPY_RESUME`` flag (``virsh migrate --postcopy-resume``) -- 2.25.1
Re: [PATCH 4/4] qemu: validate: use domcaps for tpm validation
On 6/21/22 4:11 AM, Michal Prívozník wrote:
> On 6/18/22 20:32, Cole Robinson wrote:
>> Replace tpm->type and tpm->model qemuCaps validation with the
>> similar logic in domcaps.
>>
>> Signed-off-by: Cole Robinson
>> ---
>> src/qemu/qemu_validate.c | 71 ++--
>> 1 file changed, 17 insertions(+), 54 deletions(-)
>>
>> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
>> index db47fcaa9c..39210ba65b 100644
>> --- a/src/qemu/qemu_validate.c
>> +++ b/src/qemu/qemu_validate.c
>> @@ -4750,7 +4750,7 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm,
>> const virDomainDef *def,
>> virQEMUCaps *qemuCaps)
>> {
>> -virQEMUCapsFlags flag;
>> +virDomainCapsDeviceTPM tpmCaps = { 0 };
>>
>> switch (tpm->version) {
>> case VIR_DOMAIN_TPM_VERSION_1_2:
>> @@ -4781,57 +4781,28 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm,
>> break;
>> }
>>
>> -switch (tpm->type) {
>> -case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
>> -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_TPM_PASSTHROUGH))
>> -goto no_support;
>> -break;
>> -
>> -case VIR_DOMAIN_TPM_TYPE_EMULATOR:
>> -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_TPM_EMULATOR))
>> -goto no_support;
>> +virQEMUCapsFillDomainDeviceTPMCaps(qemuCaps, );
>>
>> -break;
>> -case VIR_DOMAIN_TPM_TYPE_LAST:
>> -break;
>> +if (!VIR_DOMAIN_CAPS_ENUM_IS_SET(tpmCaps.backendModel, tpm->type)) {
>> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> + _("The QEMU executable %s does not support TPM "
>> + "backend type %s"),
>> + def->emulator,
>> + virDomainTPMBackendTypeToString(tpm->type));
>
> Whoa, very nice idea! And looking around the file I can see it used
> already. How could this slipped by me? I mean, the more I think about it
> the more possibilities for code deduplication I see. And on the flip
> side - we would be motivated to keep domcaps on the bleeding edge.
>
Heh, it's your code :)
https://listman.redhat.com/archives/libvir-list/2020-November/211844.html
But yes I agree. domcaps can be a detriment to apps if it's not
reliable, and duplicating the qemuCaps checking is always going to lead
to issues like this. It would be nice if we could normalize adding
domcaps coverage for basic qemuCaps validation cases like this.
Thanks,
Cole
Re: [PATCH 0/4] qemu: Fix tpm-tis for armv7l and riscv
On 6/18/22 20:32, Cole Robinson wrote: > This fixes tpm-tis usage for armv7l and riscv arches, and then > switches qemu tpm validation to use domcaps as the source of truth > > Cole Robinson (4): > qemu: validate: Drop tpm-tis arch validation > qemu: command: Use correct tpm device for all non-x86 > tests: mock swtpm initialization for all qemu tests > qemu: validate: use domcaps for tpm validation > > src/qemu/qemu_command.c | 2 +- > src/qemu/qemu_validate.c | 77 +--- > tests/domaincapstest.c | 7 > tests/testutilsqemu.c| 8 + > 4 files changed, 26 insertions(+), 68 deletions(-) > Reviewed-by: Michal Privoznik Michal
Re: [PATCH 4/4] qemu: validate: use domcaps for tpm validation
On 6/18/22 20:32, Cole Robinson wrote:
> Replace tpm->type and tpm->model qemuCaps validation with the
> similar logic in domcaps.
>
> Signed-off-by: Cole Robinson
> ---
> src/qemu/qemu_validate.c | 71 ++--
> 1 file changed, 17 insertions(+), 54 deletions(-)
>
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index db47fcaa9c..39210ba65b 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -4750,7 +4750,7 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm,
> const virDomainDef *def,
> virQEMUCaps *qemuCaps)
> {
> -virQEMUCapsFlags flag;
> +virDomainCapsDeviceTPM tpmCaps = { 0 };
>
> switch (tpm->version) {
> case VIR_DOMAIN_TPM_VERSION_1_2:
> @@ -4781,57 +4781,28 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm,
> break;
> }
>
> -switch (tpm->type) {
> -case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_TPM_PASSTHROUGH))
> -goto no_support;
> -break;
> -
> -case VIR_DOMAIN_TPM_TYPE_EMULATOR:
> -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_TPM_EMULATOR))
> -goto no_support;
> +virQEMUCapsFillDomainDeviceTPMCaps(qemuCaps, );
>
> -break;
> -case VIR_DOMAIN_TPM_TYPE_LAST:
> -break;
> +if (!VIR_DOMAIN_CAPS_ENUM_IS_SET(tpmCaps.backendModel, tpm->type)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("The QEMU executable %s does not support TPM "
> + "backend type %s"),
> + def->emulator,
> + virDomainTPMBackendTypeToString(tpm->type));
Whoa, very nice idea! And looking around the file I can see it used
already. How could this slipped by me? I mean, the more I think about it
the more possibilities for code deduplication I see. And on the flip
side - we would be motivated to keep domcaps on the bleeding edge.
Michal
Re: [PATCH] libxl: Fix domain startup failure error reporting
On 6/17/22 23:29, Cole Robinson wrote:
> When domain startup fails, domain cleanup calls
> libxlNetworkUnwindDevices, which calls virGetConnectNetwork, which
> is a top level API entry point, which resets the initial saved error,
> leading to clients seeing:
>
> error: An error occurred, but the cause is unknown
>
> This preserves the error from before virGetConnectNetwork is called.
>
> Signed-off-by: Cole Robinson
> ---
> src/libxl/libxl_domain.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index 17b347de4e..bda110e9e6 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -830,12 +830,17 @@ libxlNetworkUnwindDevices(virDomainDef *def)
> /* cleanup actual device */
> virDomainNetRemoveHostdev(def, net);
> if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
> -g_autoptr(virConnect) conn = virGetConnectNetwork();
> +g_autoptr(virConnect) conn = NULL;
> +virErrorPtr save_err;
> +
> +virErrorPreserveLast(_err);
> +conn = virGetConnectNetwork();
>
> if (conn)
> virDomainNetReleaseActualDevice(conn, def, net);
> else
> VIR_WARN("Unable to release network device '%s'",
> NULLSTR(net->ifname));
> +virErrorRestore(_err);
> }
> }
> }
This fixes this particular function. I wonder whether we should mimic
what QEMU driver does and wrap whole qemuProcessShutdown(), I mean
libxlDomainCleanup() in virErrorPreserveLast(). Something like this:
diff --git i/src/libxl/libxl_domain.c w/src/libxl/libxl_domain.c
index bda110e9e6..8e8ddd284a 100644
--- i/src/libxl/libxl_domain.c
+++ w/src/libxl/libxl_domain.c
@@ -908,10 +908,13 @@ libxlDomainCleanup(libxlDriverPrivate *driver,
virHostdevManager *hostdev_mgr = driver->hostdevMgr;
unsigned int hostdev_flags = VIR_HOSTDEV_SP_PCI;
size_t i;
+virErrorPtr save_err;
VIR_DEBUG("Cleaning up domain with id '%d' and name '%s'",
vm->def->id, vm->def->name);
+virErrorPreserveLast(_err);
+
hostdev_flags |= VIR_HOSTDEV_SP_USB;
/* Call hook with stopped operation. Ignore error and continue with
cleanup */
@@ -984,6 +987,7 @@ libxlDomainCleanup(libxlDriverPrivate *driver,
VIR_HOOK_SUBOP_END, NULL));
virDomainObjRemoveTransientDef(vm);
+virErrorRestore(_err);
}
/*
@@ -1245,6 +1249,7 @@ libxlDomainStartPrepare(libxlDriverPrivate *driver,
{
virHostdevManager *hostdev_mgr = driver->hostdevMgr;
unsigned int hostdev_flags = VIR_HOSTDEV_SP_PCI | VIR_HOSTDEV_SP_USB;
+virErrorPtr save_err;
if (virDomainObjSetDefTransient(driver->xmlopt, vm, NULL) < 0)
return -1;
@@ -1272,10 +1277,12 @@ libxlDomainStartPrepare(libxlDriverPrivate *driver,
return 0;
error:
+virErrorPreserveLast(_err);
libxlNetworkUnwindDevices(vm->def);
virHostdevReAttachDomainDevices(hostdev_mgr, LIBXL_DRIVER_INTERNAL_NAME,
vm->def, hostdev_flags);
virDomainObjRemoveTransientDef(vm);
+virErrorRestore(_err);
return -1;
}
If this works, replace your patch with this diff, apply my:
Reviewed-by: Michal Privoznik
and push.
Michal
Re: [libvirt PATCH v2] tools: add virt-qmp-proxy for proxying QMP clients to libvirt QEMU guests
Daniel P. Berrangé writes:
> Libvirt provides QMP passthrough APIs for the QEMU driver and these are
> exposed in virsh. It is not especially pleasant, however, using the raw
> QMP JSON syntax. QEMU has a tool 'qmp-shell' which can speak QMP and
> exposes a human friendly interactive shell. It is not possible to use
> this with libvirt managed guest, however, since only one client can
> attach to he QMP socket at any point in time.
On the other hand, you can configure multiple QMP monitors.
Regardless, users get to do with QMP what users find useful. No
objections from me.
> The virt-qmp-proxy tool aims to solve this problem. It opens a UNIX
> socket and listens for incoming client connections, speaking QMP on
> the connected socket. It will forward any QMP commands received onto
> the running libvirt QEMU guest, and forward any replies back to the
> QMP client.
>
> $ virsh start demo
> $ virt-qmp-proxy demo demo.qmp &
> $ qmp-shell demo.qmp
> Welcome to the QMP low-level shell!
> Connected to QEMU 6.2.0
>
> (QEMU) query-kvm
> {
> "return": {
> "enabled": true,
> "present": true
> }
> }
>
> Note this tool of course has the same risks as the raw libvirt
> QMP passthrough. It is safe to run query commands to fetch information
> but commands which change the QEMU state risk disrupting libvirt's
> management of QEMU, potentially resulting in data loss/corruption in
> the worst case.
>
> Signed-off-by: Daniel P. Berrangé
