[libvirt] [PATCH 2/2] Fix the build on non-linux platforms after VIR_AUTOPTR related changes
Commits 7b706f33ac and 4acb7887e4 introduced some compound type *Free wrappers in order to use them with VIR_DEFINE_AUTOPTR_FUNC. However, since those were not used in the code right away, Clang complained about unused functions (static ones that are defined by the macro above). This patch puts the defined functions in use. Signed-off-by: Erik Skultety --- src/util/virnetdev.c | 3 +-- src/util/virnetlink.c | 6 ++ 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 9eca786c95..8eac419725 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -2833,7 +2833,7 @@ static int virNetDevGetMcastList(const char *ifname, char *buf = NULL; char *next = NULL; int ret = -1, len; -virNetDevMcastEntryPtr entry = NULL; +VIR_AUTOPTR(virNetDevMcastEntry) entry = NULL; mcast->entries = NULL; mcast->nentries = 0; @@ -2867,7 +2867,6 @@ static int virNetDevGetMcastList(const char *ifname, ret = 0; cleanup: VIR_FREE(buf); -VIR_FREE(entry); return ret; } diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index 2702c12d24..162efe6f99 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -305,7 +305,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg, .nl_groups = 0, }; struct pollfd fds[1]; -virNetlinkHandle *nlhandle = NULL; +VIR_AUTOPTR(virNetlinkHandle) nlhandle = NULL; int len = 0; memset(fds, 0, sizeof(fds)); @@ -333,7 +333,6 @@ int virNetlinkCommand(struct nl_msg *nl_msg, *respbuflen = 0; } -virNetlinkFree(nlhandle); return ret; } @@ -355,7 +354,7 @@ virNetlinkDumpCommand(struct nl_msg *nl_msg, .nl_pid= dst_pid, .nl_groups = 0, }; -virNetlinkHandle *nlhandle = NULL; +VIR_AUTOPTR(virNetlinkHandle) nlhandle = NULL; if (!(nlhandle = virNetlinkSendRequest(nl_msg, src_pid, nladdr, protocol, groups))) @@ -382,7 +381,6 @@ virNetlinkDumpCommand(struct nl_msg *nl_msg, cleanup: VIR_FREE(resp); -virNetlinkFree(nlhandle); return ret; } -- 2.14.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] util: virnetdevopenvswitch: Drop an unused variable @ovs_timeout
Technically, it was never used ever since commit @f4d06ca8fd9 introduced it, but the fact that we called VIR_FREE on it was enough for Clang to never complain about it. Signed-off-by: Erik Skultety --- CC'ing Boris, as he allowed specifying timeouts for openvswitch calls, however, this particular one hasn't been in use ever. src/util/virnetdevopenvswitch.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 69ae04ee5a..9a9435f8af 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -473,7 +473,6 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path, size_t ntokens = 0; int status; int ret = -1; -VIR_AUTOFREE(char *) ovs_timeout = NULL; /* Openvswitch vhostuser path are hardcoded to * //openvswitch/ -- 2.14.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Fix the non-linux platform build after recent VIR_AUTOPTR patches
There were some complaints about an unused variable (patch 1) and unused static functions which are defined by the VIR_DEFINE_AUTOPTR_FUNC macro (patch 2). Successful Travis build: https://travis-ci.org/eskultety/libvirt/builds/413187470 MinGW wasn't affected by the recent VIR_AUTOPTR series: https://ci.centos.org/view/libvirt/job/libvirt-master-build-mingw64/330/ *NOT* pushing under the build breaker in case someone wants to have a look. Erik Skultety (2): util: virnetdevopenvswitch: Drop an unused variable @ovs_timeout Fix the build on non-linux platforms after VIR_AUTOPTR related changes src/util/virnetdev.c| 3 +-- src/util/virnetdevopenvswitch.c | 1 - src/util/virnetlink.c | 6 ++ 3 files changed, 3 insertions(+), 7 deletions(-) -- 2.14.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: ensure "pc" machine is always used as default if available
On Tue, Aug 07, 2018 at 12:17:33PM +0100, Daniel P. Berrangé wrote: > On Fri, Aug 03, 2018 at 01:05:49PM -0300, Eduardo Habkost wrote: > > On Fri, Aug 03, 2018 at 01:59:47PM +0100, Daniel P. Berrangé wrote: > > > It is increasingly likely that some distro is going to change the > > > default "x86" machine type in QEMU from "pc" to "q35". This will > > > certainly break existing applications which write their XML on the > > > assumption that its using a "pc" machine by default. For example they'll > > > lack a IDE CDROM and get PCI-X instad of PCI which changes the topology > > > radically. > > > > > > Libvirt promises to isolate applications from hypervisor changes that > > > may cause incompatibilities, so we must ensure that we always use the > > > "pc" machine type if it is available. Only use QEMU's own reported > > > default machine type if "pc" does not exist. > > > > > > Note this change assumes there will always be a "pc" alias as long as a > > > versioned "pc-XXX" machine type exists. If QEMU were to ship a "pc-XXX" > > > machine type but not provide the "pc" alias, it is too hard to decide > > > which to default so. Versioned machine types are supposed to be > > > considered opaque strings, so we can't apply any sensible ordering > > > ourselves and QEMU isn't reporting the list of machines in any sensible > > > ordering itself. > > > > > > Signed-off-by: Daniel P. Berrangé > > > > Won't this break qemuParseCommandLine() if it sees a QEMU binary > > running without "-machine"? It will assume the QEMU default is > > "pc" but this may be not true. > > If no -machine arg is present in ARGV, then the code will lookup the > default machine type for the emulator binary in the capabilities > record. So this should just "do the right thing" with my changes > in this patch. I don't see how it would do the right thing. e.g.: if we have a QEMU binary that defaults to "q35" and it is running without "-machine", it will be emulating a q35 machine, not i440fx. But with this change qemuParseCommandLine() will incorrectly assume the existing process is emulating i440fx. qemuParseCommandLine() calls: capsdata = virCapabilitiesDomainDataLookup(caps, def->os.type, def->os.arch, def->virtType, NULL, NULL) and as far as I can see, your patch will make qemuParseCommandLine() set capsdata->machinetype to a pc-i440fx-* machine, even if the QEMU binary defaults to something else. -- Eduardo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt v2 1/9] virConnectRegisterCloseCallback: Cleanup 'opaque' if there is no connectRegisterCloseCallback
On Tue, Aug 07, 2018 at 06:40:46PM +0200, Marc Hartmayer wrote: > On Wed, Jun 13, 2018 at 10:22 AM +0200, Marc Hartmayer > wrote: > > On Mon, Jun 04, 2018 at 06:25 PM +0200, "Daniel P. Berrangé" > > wrote: > >> On Thu, Apr 26, 2018 at 08:16:54PM -0400, John Ferlan wrote: > > […snip…] > > >> > >> If the application wants to access 'opaque' outside the context of the > >> callback function, it must take steps to ensure it is still alive in > >> whatever thread it using it. This implies the data passed for 'opaque' > >> should be ref-counted and they must hold a reference for their own > >> usage, separately from the reference assoicated with the callback that > >> will be released by @freecb. > >> > >> That all said, we could take a slightly different approach if we want > >> to be paranoid about this > >> > >> eg move the > >> > >> virConnectCloseCallbackDataPtr closeCallback; > >> > >> out of the driver specific private structs, and put it in the main > >> struct _virConnect instead. > > > > This sound like a revert of commit “close callback: move it to driver” > > (88f09b75eb99415c). Shall we really do this? > > Polite ping. It is mostly a revert i think Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH libvirt v2 1/9] virConnectRegisterCloseCallback: Cleanup 'opaque' if there is no connectRegisterCloseCallback
On Wed, Jun 13, 2018 at 10:22 AM +0200, Marc Hartmayer wrote: > On Mon, Jun 04, 2018 at 06:25 PM +0200, "Daniel P. Berrangé" > wrote: >> On Thu, Apr 26, 2018 at 08:16:54PM -0400, John Ferlan wrote: […snip…] >> >> If the application wants to access 'opaque' outside the context of the >> callback function, it must take steps to ensure it is still alive in >> whatever thread it using it. This implies the data passed for 'opaque' >> should be ref-counted and they must hold a reference for their own >> usage, separately from the reference assoicated with the callback that >> will be released by @freecb. >> >> That all said, we could take a slightly different approach if we want >> to be paranoid about this >> >> eg move the >> >> virConnectCloseCallbackDataPtr closeCallback; >> >> out of the driver specific private structs, and put it in the main >> struct _virConnect instead. > > This sound like a revert of commit “close callback: move it to driver” > (88f09b75eb99415c). Shall we really do this? Polite ping. > >> >> The main libvirt-host.c can add the callback to this itself. THe >> driver code only needs to worry about actually invoking the callback >> >> That would allow us to have freecb() called at the right time for >> all drivers, even if they don't ever use the close callback. >> >>> >>> I'm still not sure I understand why the API cannot return a failure, but >>> Daniel says it cannot. >> >> It can break existing applications using hypervisors that don't >> implement this API, becasue its a change in behaviour. In retrospect >> I wouldn't have written the API in this way today, but we must live >> with the design we have. >> >> >> Regards, >> Daniel >> -- >> |: https://berrange.com -o-https://www.flickr.com/photos/dberrange >> :| >> |: https://libvirt.org -o-https://fstop138.berrange.com >> :| >> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange >> :| >> > -- > Beste Grüße / Kind regards >Marc Hartmayer > > IBM Deutschland Research & Development GmbH > Vorsitzende des Aufsichtsrats: Martina Koederitz > Geschäftsführung: Dirk Wittkopp > Sitz der Gesellschaft: Böblingen > Registergericht: Amtsgericht Stuttgart, HRB 243294 -- Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 05/32] util: netdev: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
On Tue, Aug 07, 2018 at 06:04:37PM +0200, Erik Skultety wrote: > On Tue, Aug 07, 2018 at 04:14:06PM +0100, Daniel P. Berrangé wrote: > > On Sat, Jul 28, 2018 at 11:31:20PM +0530, Sukrit Bhatnagar wrote: > > > Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in > > > src/util/viralloc.h, define a new wrapper around an existing > > > cleanup function which will be called when a variable declared > > > with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant > > > viralloc.h include, since that has moved from the source module into > > > the header. > > > > > > When variables of type virNetDevRxFilterPtr and virNetDevMcastEntryPtr > > > are declared using VIR_AUTOPTR, the functions virNetDevRxFilterFree > > > and virNetDevMcastEntryFree, respectively, will be run > > > automatically on them when they go out of scope. > > > > > > Signed-off-by: Sukrit Bhatnagar > > > --- > > > src/util/virnetdev.c | 9 - > > > src/util/virnetdev.h | 4 > > > 2 files changed, 12 insertions(+), 1 deletion(-) > > > > > > diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c > > > index 0777eca..9eca786 100644 > > > --- a/src/util/virnetdev.c > > > +++ b/src/util/virnetdev.c > > > @@ -29,7 +29,6 @@ > > > #include "virfile.h" > > > #include "virerror.h" > > > #include "vircommand.h" > > > -#include "viralloc.h" > > > #include "virpci.h" > > > #include "virlog.h" > > > #include "virstring.h" > > > @@ -120,6 +119,14 @@ struct _virNetDevMcastEntry { > > > virMacAddr macaddr; > > > }; > > > > > > +static void > > > +virNetDevMcastEntryFree(virNetDevMcastEntryPtr entry) > > > +{ > > > +VIR_FREE(entry); > > > +} > > > + > > > +VIR_DEFINE_AUTOPTR_FUNC(virNetDevMcastEntry, virNetDevMcastEntryFree) > > > > This causes a compile problem on non-linux platforms due to the function > > being unused: > > > > util/virnetdev.c:128:1: error: unused function > > 'virNetDevMcastEntryAutoPtrFree' [-Werror,-Wunused-function] > > VIR_DEFINE_AUTOPTR_FUNC(virNetDevMcastEntry, virNetDevMcastEntryFree) > > ^ > > ./util/viralloc.h:612:24: note: expanded from macro > > 'VIR_DEFINE_AUTOPTR_FUNC' > > static inline void VIR_AUTOPTR_FUNC_NAME(type)(type **_ptr) \ > >^ > > ./util/viralloc.h:600:38: note: expanded from macro 'VIR_AUTOPTR_FUNC_NAME' > > # define VIR_AUTOPTR_FUNC_NAME(type) type##AutoPtrFree > > ^ > > :21:1: note: expanded from here > > virNetDevMcastEntryAutoPtrFree > > ^ > > 1 error generated. > > Sigh...yeah, there are a few of them, I had some notes in patches that > actually > used the definitions, nevertheless should have been compilable on their own > with clang in the first place. I'm already running my local fix in Travis, > I'll > be sending patches afterwards. Worth checking a mingw build too, since mingw disables some stuff that is still present on OS-X, so might tickle further code path variations. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 05/32] util: netdev: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
On Tue, Aug 07, 2018 at 04:14:06PM +0100, Daniel P. Berrangé wrote: > On Sat, Jul 28, 2018 at 11:31:20PM +0530, Sukrit Bhatnagar wrote: > > Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in > > src/util/viralloc.h, define a new wrapper around an existing > > cleanup function which will be called when a variable declared > > with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant > > viralloc.h include, since that has moved from the source module into > > the header. > > > > When variables of type virNetDevRxFilterPtr and virNetDevMcastEntryPtr > > are declared using VIR_AUTOPTR, the functions virNetDevRxFilterFree > > and virNetDevMcastEntryFree, respectively, will be run > > automatically on them when they go out of scope. > > > > Signed-off-by: Sukrit Bhatnagar > > --- > > src/util/virnetdev.c | 9 - > > src/util/virnetdev.h | 4 > > 2 files changed, 12 insertions(+), 1 deletion(-) > > > > diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c > > index 0777eca..9eca786 100644 > > --- a/src/util/virnetdev.c > > +++ b/src/util/virnetdev.c > > @@ -29,7 +29,6 @@ > > #include "virfile.h" > > #include "virerror.h" > > #include "vircommand.h" > > -#include "viralloc.h" > > #include "virpci.h" > > #include "virlog.h" > > #include "virstring.h" > > @@ -120,6 +119,14 @@ struct _virNetDevMcastEntry { > > virMacAddr macaddr; > > }; > > > > +static void > > +virNetDevMcastEntryFree(virNetDevMcastEntryPtr entry) > > +{ > > +VIR_FREE(entry); > > +} > > + > > +VIR_DEFINE_AUTOPTR_FUNC(virNetDevMcastEntry, virNetDevMcastEntryFree) > > This causes a compile problem on non-linux platforms due to the function > being unused: > > util/virnetdev.c:128:1: error: unused function > 'virNetDevMcastEntryAutoPtrFree' [-Werror,-Wunused-function] > VIR_DEFINE_AUTOPTR_FUNC(virNetDevMcastEntry, virNetDevMcastEntryFree) > ^ > ./util/viralloc.h:612:24: note: expanded from macro 'VIR_DEFINE_AUTOPTR_FUNC' > static inline void VIR_AUTOPTR_FUNC_NAME(type)(type **_ptr) \ >^ > ./util/viralloc.h:600:38: note: expanded from macro 'VIR_AUTOPTR_FUNC_NAME' > # define VIR_AUTOPTR_FUNC_NAME(type) type##AutoPtrFree > ^ > :21:1: note: expanded from here > virNetDevMcastEntryAutoPtrFree > ^ > 1 error generated. Sigh...yeah, there are a few of them, I had some notes in patches that actually used the definitions, nevertheless should have been compilable on their own with clang in the first place. I'm already running my local fix in Travis, I'll be sending patches afterwards. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 05/32] util: netdev: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
On Sat, Jul 28, 2018 at 11:31:20PM +0530, Sukrit Bhatnagar wrote: > Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in > src/util/viralloc.h, define a new wrapper around an existing > cleanup function which will be called when a variable declared > with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant > viralloc.h include, since that has moved from the source module into > the header. > > When variables of type virNetDevRxFilterPtr and virNetDevMcastEntryPtr > are declared using VIR_AUTOPTR, the functions virNetDevRxFilterFree > and virNetDevMcastEntryFree, respectively, will be run > automatically on them when they go out of scope. > > Signed-off-by: Sukrit Bhatnagar > --- > src/util/virnetdev.c | 9 - > src/util/virnetdev.h | 4 > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c > index 0777eca..9eca786 100644 > --- a/src/util/virnetdev.c > +++ b/src/util/virnetdev.c > @@ -29,7 +29,6 @@ > #include "virfile.h" > #include "virerror.h" > #include "vircommand.h" > -#include "viralloc.h" > #include "virpci.h" > #include "virlog.h" > #include "virstring.h" > @@ -120,6 +119,14 @@ struct _virNetDevMcastEntry { > virMacAddr macaddr; > }; > > +static void > +virNetDevMcastEntryFree(virNetDevMcastEntryPtr entry) > +{ > +VIR_FREE(entry); > +} > + > +VIR_DEFINE_AUTOPTR_FUNC(virNetDevMcastEntry, virNetDevMcastEntryFree) This causes a compile problem on non-linux platforms due to the function being unused: util/virnetdev.c:128:1: error: unused function 'virNetDevMcastEntryAutoPtrFree' [-Werror,-Wunused-function] VIR_DEFINE_AUTOPTR_FUNC(virNetDevMcastEntry, virNetDevMcastEntryFree) ^ ./util/viralloc.h:612:24: note: expanded from macro 'VIR_DEFINE_AUTOPTR_FUNC' static inline void VIR_AUTOPTR_FUNC_NAME(type)(type **_ptr) \ ^ ./util/viralloc.h:600:38: note: expanded from macro 'VIR_AUTOPTR_FUNC_NAME' # define VIR_AUTOPTR_FUNC_NAME(type) type##AutoPtrFree ^ :21:1: note: expanded from here virNetDevMcastEntryAutoPtrFree ^ 1 error generated. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: ensure default machine types don't change if QEMU changes
On Tue, Aug 07, 2018 at 04:57:28PM +0200, Andrea Bolognani wrote: > On Tue, 2018-08-07 at 15:28 +0100, Daniel P. Berrangé wrote: > > On Tue, Aug 07, 2018 at 03:54:06PM +0200, Andrea Bolognani wrote: > > > I wonder if we shouldn't just drop the default machine type handling > > > altogether at this point, though. > > > > That's impossible as it violates the back compatibility guarantee > > and will certainly break applications > > In abstract terms, given that the whole point of this exercise is > shielding our users from changes in QEMU, why wouldn't we go the > whole way and take QEMU defaults out of the picture entirely? > > I just can't picture a scenario where ignoring the QEMU defaults > would actually cause issues, since we're basically moving the > defaults into libvirt with this commit... Can you describe such > a scenario? Someone could build a QEMU with the "pc" machine type deleted entirely, in which case libvirt won't find the default. The best we can do there is fallback to QEMU's own default. To avoid that problem we would have to maintain a sorted list of every single known machine type in every QEMU which gets a bit ridiculous. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: ensure default machine types don't change if QEMU changes
On Tue, 2018-08-07 at 15:28 +0100, Daniel P. Berrangé wrote: > On Tue, Aug 07, 2018 at 03:54:06PM +0200, Andrea Bolognani wrote: > > I wonder if we shouldn't just drop the default machine type handling > > altogether at this point, though. > > That's impossible as it violates the back compatibility guarantee > and will certainly break applications In abstract terms, given that the whole point of this exercise is shielding our users from changes in QEMU, why wouldn't we go the whole way and take QEMU defaults out of the picture entirely? I just can't picture a scenario where ignoring the QEMU defaults would actually cause issues, since we're basically moving the defaults into libvirt with this commit... Can you describe such a scenario? -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 00/32] use GNU C's cleanup attribute in src/util (batch III)
On Sat, Jul 28, 2018 at 11:31:15PM +0530, Sukrit Bhatnagar wrote: > This third series of patches also modifies a few files in src/util > to use VIR_AUTOFREE and VIR_AUTOPTR for automatic freeing of memory > and get rid of some VIR_FREE macro invocations and *Free function > calls. So I went ahead an pushed most of the patches (some got tiny adjustments as I noted in the review, I split some of them in 2 when there was a function being defined at the same time as a usage of the VIR_DEFINE_AUTOPTR_FUNC). Following are the patches where there were some issues that need to be worked on (like the virBuffer leak) or there was something missing and therefore are not pushed: 3, 6, 7, 10, 13, 14, 18, 23. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/9] docs: move storage initiator def into storagecommon.rng
On Tue, Aug 07, 2018 at 03:55:25PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- docs/schemas/storagecommon.rng | 11 +++ docs/schemas/storagepool.rng | 11 --- 2 files changed, 11 insertions(+), 11 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: ensure default machine types don't change if QEMU changes
On Tue, Aug 07, 2018 at 03:54:06PM +0200, Andrea Bolognani wrote: > On Tue, 2018-08-07 at 13:22 +0100, Daniel P. Berrangé wrote: > [...] > > + * Our preferred pmachine might have been compiled out of > > s/pmachine/machine/ > > > + * QEMU at build time though, so we still fallback to honouring > > + * QEMU's reported default in that case > > + */ > > +if (preferredIdx == -1) > > +preferredIdx = defIdx; > > +if (preferredIdx != -1) > > +virQEMUCapsSetDefaultMachine(qemuCaps, preferredIdx); > > This is certainly an improvement compared to the current situation, > so > > Reviewed-by: Andrea Bolognani > > I wonder if we shouldn't just drop the default machine type handling > altogether at this point, though. That's impossible as it violates the back compatibility guarantee and will certainly break applications Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/9] conf: use virStorageSourceInitiator functions
On Tue, Aug 07, 2018 at 03:55:24PM +0200, Pavel Hrdina wrote: Signed-off-by: Pavel Hrdina --- src/conf/storage_conf.c | 16 +--- 1 file changed, 5 insertions(+), 11 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/9] util: introduce virStorageSourceInitiator functions
On Tue, Aug 07, 2018 at 03:55:23PM +0200, Pavel Hrdina wrote: The same code would be used for storage pools and domain disks. Signed-off-by: Pavel Hrdina --- src/util/virstoragefile.c | 34 ++ src/util/virstoragefile.h | 15 +++ 2 files changed, 49 insertions(+) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/9] conf: rename and move virStoragePoolSourceInitiatorAttr
On Tue, Aug 07, 2018 at 03:55:22PM +0200, Pavel Hrdina wrote: This structure will be reused by domain disk images as well. Signed-off-by: Pavel Hrdina --- src/conf/storage_conf.h | 7 +-- src/util/virstoragefile.h | 6 ++ 2 files changed, 7 insertions(+), 6 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/9] conf: properly translate iscsi-direct storage pool
On Tue, Aug 07, 2018 at 03:55:21PM +0200, Pavel Hrdina wrote: We cannot simply used the same code as for iscsi storage pool because the default mode is 'host' which is not possible with iscsi-direct. Signed-off-by: Pavel Hrdina --- src/conf/domain_conf.c | 61 +- 1 file changed, 43 insertions(+), 18 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7dcbe8a20b..d1504610a1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30143,6 +30143,34 @@ virDomainDiskTranslateSourcePoolAuth(virDomainDiskDefPtr def, } +static int +virDomainDiskTranslateISCSIDircect(virDomainDiskDefPtr def, s/Dircect/Direct/ + virStoragePoolDefPtr pooldef) +{ +def->src->srcpool->actualtype = VIR_STORAGE_TYPE_NETWORK; +def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; + Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 53/55] qemu: driver: Allow using blockdev with qemuDomainBlocksStatsGather
Use the 'qdev' instead of the disk alias to lookup the stats and transfer the capacity from the appropriate node name so that the function works with -blockdev. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 45 +++-- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6ee826029b..7309ce92e4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11077,11 +11077,13 @@ qemuDomainBlocksStatsGather(virQEMUDriverPtr driver, qemuBlockStatsPtr *retstats) { qemuDomainObjPrivatePtr priv = vm->privateData; +bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); virDomainDiskDefPtr disk; virHashTablePtr blockstats = NULL; qemuBlockStatsPtr stats; size_t i; int nstats; +int rc = 0; const char *entryname = NULL; int ret = -1; @@ -11091,23 +11093,30 @@ qemuDomainBlocksStatsGather(virQEMUDriverPtr driver, goto cleanup; } -if (!disk->info.alias) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _("missing disk device alias name for %s"), disk->dst); -goto cleanup; -} +if (blockdev) { +entryname = QEMU_DOMAIN_DISK_PRIVATE(disk)->backendQomName; +} else { +if (!disk->info.alias) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("missing disk device alias name for %s"), disk->dst); +goto cleanup; +} -entryname = disk->info.alias; +entryname = disk->info.alias; +} } qemuDomainObjEnterMonitor(driver, vm); nstats = qemuMonitorGetAllBlockStatsInfo(priv->mon, &blockstats, false); -if (capacity && nstats >= 0 && -qemuMonitorBlockStatsUpdateCapacity(priv->mon, blockstats, false) < 0) -nstats = -1; +if (capacity && nstats >= 0) { +if (blockdev) +rc = qemuMonitorBlockStatsUpdateCapacityBlockdev(priv->mon, blockstats); +else +rc = qemuMonitorBlockStatsUpdateCapacity(priv->mon, blockstats, false); +} -if (qemuDomainObjExitMonitor(driver, vm) < 0 || nstats < 0) +if (qemuDomainObjExitMonitor(driver, vm) < 0 || nstats < 0 || rc < 0) goto cleanup; if (VIR_ALLOC(*retstats) < 0) @@ -11120,12 +11129,28 @@ qemuDomainBlocksStatsGather(virQEMUDriverPtr driver, goto cleanup; } +if (blockdev) { +/* capacity are reported only per node-name so we need to transfer them */ +qemuBlockStatsPtr capstats; + +if ((capstats = virHashLookup(blockstats, disk->src->nodeformat))) { +(*retstats)->capacity = capstats->capacity; +(*retstats)->physical = capstats->physical; +(*retstats)->wr_highest_offset = capstats->wr_highest_offset; +(*retstats)->wr_highest_offset_valid = capstats->wr_highest_offset_valid; +(*retstats)->write_threshold = capstats->write_threshold; +} +} + **retstats = *stats; } else { for (i = 0; i < vm->def->ndisks; i++) { disk = vm->def->disks[i]; entryname = disk->info.alias; +if (blockdev) +entryname = QEMU_DOMAIN_DISK_PRIVATE(disk)->backendQomName; + if (!entryname) continue; -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 55/55] DO NOT APPLY: Enable QEMU_CAPS_BLOCKDEV if 'query-blockstats' works with -blockdev
'query-blockstats' did not report the 'qdev' field prior to fixing it so that it works with our setup. Use that as a witness to enable -blockdev Note that the blockjobs were NOT adapted yet so any blockjob will desync the state of qemu. This patch needs to wait until blockjobs are fixed. --- src/qemu/qemu_capabilities.c | 1 + tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml | 1 + tests/qemuxml2argvdata/disk-aio.x86_64-latest.args | 19 ++- .../disk-backing-chains-noindex.x86_64-latest.args | 145 ++--- .../qemuxml2argvdata/disk-cache.x86_64-latest.args | 50 --- .../disk-cdrom-network.x86_64-latest.args | 32 +++-- .../disk-cdrom-tray.x86_64-latest.args | 24 ++-- .../qemuxml2argvdata/disk-cdrom.x86_64-latest.args | 17 ++- .../disk-copy_on_read.x86_64-latest.args | 19 ++- .../disk-detect-zeroes.x86_64-latest.args | 17 ++- .../disk-error-policy.x86_64-latest.args | 30 +++-- .../disk-network-gluster.x86_64-latest.args| 32 +++-- .../disk-network-iscsi.x86_64-latest.args | 58 + .../disk-network-nbd.x86_64-latest.args| 41 -- .../disk-network-rbd.x86_64-latest.args| 67 ++ .../disk-network-sheepdog.x86_64-latest.args | 16 ++- .../disk-network-source-auth.x86_64-latest.args| 30 +++-- .../disk-network-tlsx509.x86_64-latest.args| 61 + .../disk-readonly-disk.x86_64-latest.args | 14 +- .../disk-shared.x86_64-latest.args | 18 ++- ...isk-virtio-scsi-reservations.x86_64-latest.args | 20 ++- 21 files changed, 494 insertions(+), 218 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6b4c14ac50..58512685bf 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1309,6 +1309,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = { { "blockdev-add/arg-type/+qcow2/encrypt/+luks/key-secret", QEMU_CAPS_QCOW2_LUKS }, { "nbd-server-start/arg-type/tls-creds", QEMU_CAPS_NBD_TLS }, { "screendump/arg-type/device", QEMU_CAPS_SCREENDUMP_DEVICE }, +{ "query-blockstats/ret-type/qdev", QEMU_CAPS_BLOCKDEV }, }; typedef struct _virQEMUCapsObjectTypeProps virQEMUCapsObjectTypeProps; diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml index a2c6085de7..8e0f2fc3b5 100644 --- a/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml @@ -216,6 +216,7 @@ + 2012093 0 427343 diff --git a/tests/qemuxml2argvdata/disk-aio.x86_64-latest.args b/tests/qemuxml2argvdata/disk-aio.x86_64-latest.args index 3894ed2502..ae82ba52bd 100644 --- a/tests/qemuxml2argvdata/disk-aio.x86_64-latest.args +++ b/tests/qemuxml2argvdata/disk-aio.x86_64-latest.args @@ -24,13 +24,20 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -no-acpi \ -boot strict=on \ -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ --drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-ide0-0-0,\ -cache=none,aio=native \ --device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1,\ +-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1",\ +"aio":"native","node-name":"libvirt-2-storage","cache":{"direct":true,\ +"no-flush":false},"read-only":false,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-2-format","read-only":false,\ +"cache":{"direct":true,"no-flush":false},"driver":"qcow2",\ +"file":"libvirt-2-storage"}' \ +-device ide-hd,bus=ide.0,unit=0,drive=libvirt-2-format,id=ide0-0-0,bootindex=1,\ write-cache=on \ --drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-1-0,\ -readonly=on,aio=threads \ --device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \ +-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest2",\ +"aio":"threads","node-name":"libvirt-1-storage","read-only":true,\ +"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":true,"driver":"raw",\ +"file":"libvirt-1-storage"}' \ +-device ide-cd,bus=ide.1,unit=0,drive=libvirt-1-format,id=ide0-1-0 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ resourcecontrol=deny \ diff --git a/tests/qemuxml2argvdata/disk-backing-chains-noindex.x86_64-latest.args b/tests/qemuxml2argvdata/disk-backing-chains-noindex.x86_64-latest.args index afe078adcc..11d805f3d8 100644 --- a/tests/qemuxml2argvdata/disk-backing-chains-noindex.x86_64-latest.args +++ b/tests/qemuxml2argvdata/disk-backing-chains-noindex.x86_64-latest.args @@ -24,33 +24,138 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ -no-acpi \ -boot strict=on \ -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ --drive 'file=gluster+unix:///Volume2/Image?socket=/path/to/sock,file.debug=4,\ -format=qcow2,if=no
[libvirt] [PATCH 50/55] qemu: Export stats relevant for the storage backend
Data relevant for the storage of a backing chain member will need to be reported separately when switching to blockdev. Prepare a function that extracts the appropriate data. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 28 +++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ee401eb702..63c1a85d5a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20128,8 +20128,29 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver, } } +ret = 0; + cleanup: +return ret; +} + + +static int +qemuDomainGetStatsBlockExportBackendStorage(const char *entryname, +virHashTablePtr stats, +size_t recordnr, +virDomainStatsRecordPtr records, +int *nrecords) +{ +qemuBlockStats *entry; +int ret = -1; + +if (!stats || !entryname || !(entry = virHashLookup(stats, entryname))) { +ret = 0; +goto cleanup; +} + if (entry->write_threshold) -QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx, "threshold", +QEMU_ADD_BLOCK_PARAM_ULL(records, nrecords, recordnr, "threshold", entry->write_threshold); ret = 0; @@ -20235,6 +20256,11 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, stats) < 0) goto cleanup; +if (qemuDomainGetStatsBlockExportBackendStorage(alias, +stats, *recordnr, +records, nrecords) < 0) +goto cleanup; + VIR_FREE(alias); (*recordnr)++; -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 45/55] qemu: Extract exporting of disk block statistics
Split out the code which converts the stats gathered in qemuDomainGetStatsBlock into typed parameters so that it will look less ugly when extending it. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 73 +- 1 file changed, 49 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b0ac5e536b..ddd991546a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20164,6 +20164,51 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver, } +static int +qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, + virHashTablePtr stats, + virHashTablePtr nodestats, + virDomainStatsRecordPtr records, + int *nrecords, + size_t *recordnr, + bool visitBacking, + virQEMUDriverPtr driver, + virQEMUDriverConfigPtr cfg, + virDomainObjPtr dom) + +{ +char *alias = NULL; +virStorageSourcePtr src = disk->src; +int ret = -1; + +while (virStorageSourceIsBacking(src) && + (src == disk->src || visitBacking)) { + +/* alias may be NULL if the VM is not running */ +if (disk->info.alias && +!(alias = qemuDomainStorageAlias(disk->info.alias, src->id))) +goto cleanup; + +qemuDomainGetStatsOneBlockRefreshNamed(src, alias, stats, nodestats); + +if (qemuDomainGetStatsOneBlock(driver, cfg, dom, records, nrecords, + disk->dst, alias, src, *recordnr, + stats) < 0) +goto cleanup; + +VIR_FREE(alias); +(*recordnr)++; +src = src->backingStore; +} + +ret = 0; + + cleanup: +VIR_FREE(alias); +return ret; +} + + static int qemuDomainGetStatsBlock(virQEMUDriverPtr driver, virDomainObjPtr dom, @@ -20184,7 +20229,6 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, int count_index = -1; size_t visited = 0; bool visitBacking = !!(privflags & QEMU_DOMAIN_STATS_BACKING); -char *alias = NULL; if (HAVE_JOB(privflags) && virDomainObjIsActive(dom)) { qemuDomainObjEnterMonitor(driver, dom); @@ -20216,35 +20260,16 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, QEMU_ADD_COUNT_PARAM(record, maxparams, "block", 0); for (i = 0; i < dom->def->ndisks; i++) { -virDomainDiskDefPtr disk = dom->def->disks[i]; -virStorageSourcePtr src = disk->src; - -while (virStorageSourceIsBacking(src) && - (src == disk->src || visitBacking)) { - -/* alias may be NULL if the VM is not running */ -if (disk->info.alias && -!(alias = qemuDomainStorageAlias(disk->info.alias, src->id))) -goto cleanup; - -qemuDomainGetStatsOneBlockRefreshNamed(src, alias, stats, nodestats); - -if (qemuDomainGetStatsOneBlock(driver, cfg, dom, record, maxparams, - disk->dst, alias, src, visited, - stats) < 0) -goto cleanup; - -VIR_FREE(alias); -visited++; -src = src->backingStore; -} +if (qemuDomainGetStatsBlockExportDisk(dom->def->disks[i], stats, nodestats, + record, maxparams, &visited, + visitBacking, driver, cfg, dom) < 0) +goto cleanup; } record->params[count_index].value.ui = visited; ret = 0; cleanup: -VIR_FREE(alias); virHashFree(stats); virHashFree(nodestats); virJSONValueFree(nodedata); -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 43/55] qemu: driver: Don't pass 'virDomainDiskDefPtr' to qemuDomainGetStatsOneBlock
Allow reuse of qemuDomainGetStatsOneBlock to work with nodenames by removing the code that looks up the stats data to the caller. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 24 +++- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index af347c732c..ed0c563c85 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20075,7 +20075,8 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver, virDomainObjPtr dom, virDomainStatsRecordPtr record, int *maxparams, - virDomainDiskDefPtr disk, + const char *diskdst, + const char *entryname, virStorageSourcePtr src, size_t block_idx, virHashTablePtr stats, @@ -20083,13 +20084,9 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver, { qemuBlockStats *entry; int ret = -1; -char *alias = NULL; -if (disk->info.alias) -alias = qemuDomainStorageAlias(disk->info.alias, src->id); +QEMU_ADD_NAME_PARAM(record, maxparams, "block", "name", block_idx, diskdst); -QEMU_ADD_NAME_PARAM(record, maxparams, "block", "name", block_idx, -disk->dst); if (virStorageSourceIsLocalStorage(src) && src->path) QEMU_ADD_NAME_PARAM(record, maxparams, "block", "path", block_idx, src->path); @@ -20108,7 +20105,7 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver, /* In case where qemu didn't provide the stats we stop here rather than * trying to refresh the stats from the disk. Inability to provide stats is * usually caused by blocked storage so this would make libvirtd hang */ -if (!stats || !alias || !(entry = virHashLookup(stats, alias))) { +if (!stats || !entryname || !(entry = virHashLookup(stats, entryname))) { ret = 0; goto cleanup; } @@ -20154,7 +20151,6 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver, ret = 0; cleanup: -VIR_FREE(alias); return ret; } @@ -20179,6 +20175,7 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, int count_index = -1; size_t visited = 0; bool visitBacking = !!(privflags & QEMU_DOMAIN_STATS_BACKING); +char *alias = NULL; if (HAVE_JOB(privflags) && virDomainObjIsActive(dom)) { qemuDomainObjEnterMonitor(driver, dom); @@ -20215,10 +20212,18 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, while (virStorageSourceIsBacking(src) && (src == disk->src || visitBacking)) { + +/* alias may be NULL if the VM is not running */ +if (disk->info.alias && +!(alias = qemuDomainStorageAlias(disk->info.alias, src->id))) +goto cleanup; + if (qemuDomainGetStatsOneBlock(driver, cfg, dom, record, maxparams, - disk, src, visited, + disk->dst, alias, src, visited, stats, nodestats) < 0) goto cleanup; + +VIR_FREE(alias); visited++; src = src->backingStore; } @@ -20228,6 +20233,7 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, ret = 0; cleanup: +VIR_FREE(alias); virHashFree(stats); virHashFree(nodestats); virJSONValueFree(nodedata); -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 35/55] qemu: driver: Prepare qemuDomainBlockResize for blockdev
Use the nodename to resize the device rather than the drive alias. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 18 +++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f994eb1538..7541beb718 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10959,6 +10959,7 @@ qemuDomainBlockResize(virDomainPtr dom, qemuDomainObjPrivatePtr priv; int ret = -1; char *device = NULL; +const char *nodename = NULL; virDomainDiskDefPtr disk = NULL; virCheckFlags(VIR_DOMAIN_BLOCK_RESIZE_BYTES, -1); @@ -11001,11 +11002,22 @@ qemuDomainBlockResize(virDomainPtr dom, disk->src->format == VIR_STORAGE_FILE_QED) size = VIR_ROUND_UP(size, 512); -if (!(device = qemuAliasDiskDriveFromDisk(disk))) -goto endjob; +if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) { +if (virStorageSourceIsEmpty(disk->src) || disk->src->readonly) { +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("can't resize empty or readonly disk '%s'"), + disk->dst); +goto endjob; +} + +nodename = disk->src->nodeformat; +} else { +if (!(device = qemuAliasDiskDriveFromDisk(disk))) +goto endjob; +} qemuDomainObjEnterMonitor(driver, vm); -if (qemuMonitorBlockResize(priv->mon, device, NULL, size) < 0) { +if (qemuMonitorBlockResize(priv->mon, device, nodename, size) < 0) { ignore_value(qemuDomainObjExitMonitor(driver, vm)); goto endjob; } -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 13/55] tests: qemu: Add test data for backing chains and indexes
Add test data for nested backing chains with/without indexes (used in status XMLs) which will excercise blockdev and the related work. Signed-off-by: Peter Krempa --- .../disk-backing-chains-index.x86_64-2.12.0.args | 1 + .../disk-backing-chains-index.x86_64-latest.args | 1 + .../qemuxml2argvdata/disk-backing-chains-index.xml | 145 + .../disk-backing-chains-noindex.x86_64-2.12.0.args | 58 + .../disk-backing-chains-noindex.x86_64-latest.args | 58 + .../disk-backing-chains-noindex.xml| 145 + tests/qemuxml2argvtest.c | 4 + .../disk-backing-chains-index-active.xml | 76 +++ .../disk-backing-chains-index-inactive.xml | 76 +++ .../disk-backing-chains-noindex-active.xml | 76 +++ .../disk-backing-chains-noindex-inactive.xml | 76 +++ tests/qemuxml2xmltest.c| 2 + 12 files changed, 718 insertions(+) create mode 12 tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-2.12.0.args create mode 12 tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-backing-chains-index.xml create mode 100644 tests/qemuxml2argvdata/disk-backing-chains-noindex.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-backing-chains-noindex.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-backing-chains-noindex.xml create mode 100644 tests/qemuxml2xmloutdata/disk-backing-chains-index-active.xml create mode 100644 tests/qemuxml2xmloutdata/disk-backing-chains-index-inactive.xml create mode 100644 tests/qemuxml2xmloutdata/disk-backing-chains-noindex-active.xml create mode 100644 tests/qemuxml2xmloutdata/disk-backing-chains-noindex-inactive.xml diff --git a/tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-2.12.0.args b/tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-2.12.0.args new file mode 12 index 00..3f4cd9040d --- /dev/null +++ b/tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-2.12.0.args @@ -0,0 +1 @@ +disk-backing-chains-noindex.x86_64-2.12.0.args \ No newline at end of file diff --git a/tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-latest.args b/tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-latest.args new file mode 12 index 00..549eb65512 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-latest.args @@ -0,0 +1 @@ +disk-backing-chains-noindex.x86_64-latest.args \ No newline at end of file diff --git a/tests/qemuxml2argvdata/disk-backing-chains-index.xml b/tests/qemuxml2argvdata/disk-backing-chains-index.xml new file mode 100644 index 00..95b8a64cf8 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-backing-chains-index.xml @@ -0,0 +1,145 @@ + + QEMUGuest1 + c7a5fdbd-edaf-9455-926a-d65c16db1809 + 219136 + 219136 + 1 + +hvm + + + + destroy + restart + destroy + +/usr/bin/qemu-system-i686 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/qemuxml2argvdata/disk-backing-chains-noindex.x86_64-2.12.0.args b/tests/qemuxml2argvdata/disk-backing-chains-noindex.x86_64-2.12.0.args new file mode 100644 index 00..dea109b13a --- /dev/null +++ b/tests/qemuxml2
[libvirt] [PATCH 24/55] qemu: Use proper backingIndex when reporting stats for backing chain
Use the index stored in virStorageSource struct rather than recalculating it. Currently we'd report proper numbers but that will change with blockdev. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 13 + 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9ee8d33f8d..8e941d7a60 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20047,7 +20047,6 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver, virDomainDiskDefPtr disk, virStorageSourcePtr src, size_t block_idx, - unsigned int backing_idx, virHashTablePtr stats, virHashTablePtr nodedata) { @@ -20056,16 +20055,16 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver, char *alias = NULL; if (disk->info.alias) -alias = qemuDomainStorageAlias(disk->info.alias, backing_idx); +alias = qemuDomainStorageAlias(disk->info.alias, src->id); QEMU_ADD_NAME_PARAM(record, maxparams, "block", "name", block_idx, disk->dst); if (virStorageSourceIsLocalStorage(src) && src->path) QEMU_ADD_NAME_PARAM(record, maxparams, "block", "path", block_idx, src->path); -if (backing_idx) +if (src->id) QEMU_ADD_BLOCK_PARAM_UI(record, maxparams, block_idx, "backingIndex", -backing_idx); +src->id); /* the VM is offline so we have to go and load the stast from the disk by * ourselves */ @@ -20182,16 +20181,14 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, for (i = 0; i < dom->def->ndisks; i++) { virDomainDiskDefPtr disk = dom->def->disks[i]; virStorageSourcePtr src = disk->src; -unsigned int backing_idx = 0; while (virStorageSourceIsBacking(src) && - (backing_idx == 0 || visitBacking)) { + (src == disk->src || visitBacking)) { if (qemuDomainGetStatsOneBlock(driver, cfg, dom, record, maxparams, - disk, src, visited, backing_idx, + disk, src, visited, stats, nodestats) < 0) goto cleanup; visited++; -backing_idx++; src = src->backingStore; } } -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 38/55] qemu: hotplug: Implement removable media change for -blockdev
Use the new APIs which allow to manipulate the tray and media separately and also allow using a nodename to refer to a media to implement media changing. With the new approach we don't have to call eject twice as the media is removed by calling qemuMonitorBlockdevMediumRemove. Signed-off-by: Peter Krempa --- src/qemu/qemu_hotplug.c | 93 - 1 file changed, 92 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 3eddb0043e..97a486f2ea 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -618,6 +618,93 @@ qemuHotplugDiskSourceRemove(qemuMonitorPtr mon, } +/** + * qemuDomainChangeMediaBlockdev: + * @driver: qemu driver structure + * @vm: domain definition + * @disk: disk definition to change the source of + * @newsrc: new disk source to change to + * @force: force the change of media + * + * Change the media in an ejectable device to the one described by + * @newsrc. This function also removes the old source from the + * shared device table if appropriate. Note that newsrc is consumed + * on success and the old source is freed on success. + * + * Returns 0 on success, -1 on error and reports libvirt error + */ +static int +qemuDomainChangeMediaBlockdev(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + virStorageSourcePtr newsrc, + bool force) +{ +qemuDomainObjPrivatePtr priv = vm->privateData; +qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); +qemuHotplugDiskSourceDataPtr newbackend = NULL; +qemuHotplugDiskSourceDataPtr oldbackend = NULL; +virStorageSourcePtr oldsrc = disk->src; +char *nodename = NULL; +int rc; +int ret = -1; + +if (!virStorageSourceIsEmpty(disk->src) && +!(oldbackend = qemuHotplugDiskSourceRemovePrepare(disk, priv->qemuCaps))) +goto cleanup; + +disk->src = newsrc; +if (!virStorageSourceIsEmpty(disk->src)) { +if (!(newbackend = qemuHotplugDiskSourceAttachPrepare(disk, + priv->qemuCaps))) +goto cleanup; + +if (qemuDomainDiskGetBackendAlias(disk, priv->qemuCaps, &nodename) < 0) +goto cleanup; +} + +if (diskPriv->tray && disk->tray_status != VIR_DOMAIN_DISK_TRAY_OPEN) { +qemuDomainObjEnterMonitor(driver, vm); +rc = qemuMonitorBlockdevTrayOpen(priv->mon, diskPriv->backendQomName, force); +if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) +goto cleanup; + +if (!force && qemuHotplugWaitForTrayEject(vm, disk) < 0) +goto cleanup; +} + +qemuDomainObjEnterMonitor(driver, vm); + +rc = qemuMonitorBlockdevMediumRemove(priv->mon, diskPriv->backendQomName); + +if (rc == 0 && oldbackend) +qemuHotplugDiskSourceRemove(priv->mon, oldbackend); + +if (newbackend && nodename) { +if (rc == 0) +rc = qemuHotplugDiskSourceAttach(priv->mon, newbackend); + +if (rc == 0) +rc = qemuMonitorBlockdevMediumInsert(priv->mon, + diskPriv->backendQomName, + nodename); +} + +if (rc == 0) +rc = qemuMonitorBlockdevTrayClose(priv->mon, diskPriv->backendQomName); + +if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) +goto cleanup; + + cleanup: +qemuHotplugDiskSourceDataFree(newbackend); +qemuHotplugDiskSourceDataFree(oldbackend); +/* caller handles correct exchange of sources */ +disk->src = oldsrc; +return ret; +} + + /** * qemuDomainChangeEjectableMedia: * @driver: qemu driver structure @@ -640,6 +727,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, virStorageSourcePtr newsrc, bool force) { +qemuDomainObjPrivatePtr priv = vm->privateData; int ret = -1; int rc; @@ -649,7 +737,10 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, if (qemuHotplugAttachManagedPR(driver, vm, newsrc, QEMU_ASYNC_JOB_NONE) < 0) goto cleanup; -rc = qemuDomainChangeMediaLegacy(driver, vm, disk, newsrc, force); +if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) +rc = qemuDomainChangeMediaBlockdev(driver, vm, disk, newsrc, force); +else +rc = qemuDomainChangeMediaLegacy(driver, vm, disk, newsrc, force); virDomainAuditDisk(vm, disk->src, newsrc, "update", rc >= 0); -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 26/55] qemu: alias: Generate QDEV name of the block backend for disks
When we stop using -drive qemu stops reporting it in some of the monitor commands. To allow referring the disk frontends and the corresponding block backends we need to know these names. Unfortunately different buses require different names. Signed-off-by: Peter Krempa --- src/qemu/qemu_alias.c | 86 +++-- src/qemu/qemu_alias.h | 3 +- src/qemu/qemu_hotplug.c | 2 +- 3 files changed, 65 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 80d9b6cf46..be3663f8d2 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -175,44 +175,82 @@ qemuAssignDeviceControllerAlias(virDomainDefPtr domainDef, } -/* Our custom -drive naming scheme used with id= */ int qemuAssignDeviceDiskAlias(virDomainDefPtr def, - virDomainDiskDefPtr disk) + virDomainDiskDefPtr disk, + virQEMUCapsPtr qemuCaps) { +qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); const char *prefix = virDomainDiskBusTypeToString(disk->bus); int controllerModel = -1; if (disk->info.alias) return 0; -if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { -if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) { -controllerModel = qemuDomainFindSCSIControllerModel(def, -&disk->info); -if (controllerModel < 0) +if (!disk->info.alias) { +if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) { +if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) { +controllerModel = qemuDomainFindSCSIControllerModel(def, + &disk->info); +if (controllerModel < 0) +return -1; +} + +if (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI || +controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) { +if (virAsprintf(&disk->info.alias, "%s%d-%d-%d", prefix, +disk->info.addr.drive.controller, +disk->info.addr.drive.bus, +disk->info.addr.drive.unit) < 0) +return -1; +} else { +if (virAsprintf(&disk->info.alias, "%s%d-%d-%d-%d", prefix, +disk->info.addr.drive.controller, +disk->info.addr.drive.bus, +disk->info.addr.drive.target, +disk->info.addr.drive.unit) < 0) +return -1; +} +} else { +int idx = virDiskNameToIndex(disk->dst); +if (virAsprintf(&disk->info.alias, "%s-disk%d", prefix, idx) < 0) return -1; } +} -if (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI || -controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) { -if (virAsprintf(&disk->info.alias, "%s%d-%d-%d", prefix, -disk->info.addr.drive.controller, -disk->info.addr.drive.bus, -disk->info.addr.drive.unit) < 0) +/* For -blockdev we need to know the QDEV ids of the block backend of the + * disk. The QDEV id used by qemu is based on the alias so we generate them + * here. */ +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) { +switch ((virDomainDiskBus) disk->bus) { +case VIR_DOMAIN_DISK_BUS_IDE: +case VIR_DOMAIN_DISK_BUS_SATA: +case VIR_DOMAIN_DISK_BUS_SCSI: +if (VIR_STRDUP(diskPriv->backendQomName, disk->info.alias) < 0) return -1; -} else { -if (virAsprintf(&disk->info.alias, "%s%d-%d-%d-%d", prefix, -disk->info.addr.drive.controller, -disk->info.addr.drive.bus, -disk->info.addr.drive.target, -disk->info.addr.drive.unit) < 0) +break; + +case VIR_DOMAIN_DISK_BUS_VIRTIO: +if (virAsprintf(&diskPriv->backendQomName, +"/machine/peripheral/%s/virtio-backend", +disk->info.alias) < 0) return -1; +break; + +case VIR_DOMAIN_DISK_BUS_USB: +if (virAsprintf(&diskPriv->backendQomName, +"/machine/peripheral/%s/%s.0/legacy[0]", +disk->info.alias, disk->info.alias) < 0) +return -1; +break; + +case VIR_DOMAIN_DISK_BUS_FDC: +case VIR_DOMAIN_DISK_BUS_XEN: +case VIR_DOMAIN_DISK_BUS_UML: +case VIR_DOMAIN_DISK_BUS_SD: +case VIR_DOMAIN_DISK_BUS_LAST: +break; } -} else { -
[libvirt] [PATCH 19/55] qemu: process: Don't detect nodenames when we support -blockdev
We'll specify them ourselves so it's pointless to attempt to redetect them. Signed-off-by: Peter Krempa --- src/qemu/qemu_process.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index b01bb4fb5c..0a072c990d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7807,7 +7807,8 @@ qemuProcessReconnect(void *opaque) if (qemuProcessRefreshDisks(driver, obj, QEMU_ASYNC_JOB_NONE) < 0) goto error; -if (qemuBlockNodeNamesDetect(driver, obj, QEMU_ASYNC_JOB_NONE) < 0) +if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV) && +qemuBlockNodeNamesDetect(driver, obj, QEMU_ASYNC_JOB_NONE) < 0) goto error; if (qemuRefreshVirtioChannelState(driver, obj, QEMU_ASYNC_JOB_NONE) < 0) -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 29/55] qemu: block: Add generator for the 'copy-on-read' blockdev driver
The copy on read functionality is done using a separate layer in the backing chain. Add function to generate properties for it. Signed-off-by: Peter Krempa --- src/qemu/qemu_block.c | 22 ++ src/qemu/qemu_block.h | 2 ++ 2 files changed, 24 insertions(+) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 509b0a5210..d8020eecbe 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1743,3 +1743,25 @@ qemuBlockSnapshotAddLegacy(virJSONValuePtr actions, VIR_FREE(source); return ret; } + + +/** + * qemuBlockStorageGetCopyOnReadProps: + * @disk: disk with copy-on-read enabled + * + * Creates blockdev properties for a disk copy-on-read layer. + */ +virJSONValuePtr +qemuBlockStorageGetCopyOnReadProps(virDomainDiskDefPtr disk) +{ +qemuDomainDiskPrivatePtr priv = QEMU_DOMAIN_DISK_PRIVATE(disk); +virJSONValuePtr ret = NULL; + +ignore_value(virJSONValueObjectCreate(&ret, + "s:driver", "copy-on-read", + "s:node-name", priv->nodeCopyOnRead, + "s:file", disk->src->nodeformat, + NULL)); + +return ret; +} diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index fd8984e60b..62ed5027cb 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -67,6 +67,8 @@ qemuBlockStorageSourceGetURI(virStorageSourcePtr src); virJSONValuePtr qemuBlockStorageSourceGetBlockdevProps(virStorageSourcePtr src); +virJSONValuePtr +qemuBlockStorageGetCopyOnReadProps(virDomainDiskDefPtr disk); typedef struct qemuBlockStorageSourceAttachData qemuBlockStorageSourceAttachData; typedef qemuBlockStorageSourceAttachData *qemuBlockStorageSourceAttachDataPtr; -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 28/55] qemu: proces: assign node names for user defined backing chains
Prepare the full backing chain by instantiating authentication and TLS transport secrets and other necessary objects so that we can add the full backing chain explicitly to qemu. This also includes allocation of nodenames for the individual backing chain members. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 76 -- 1 file changed, 74 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c98be208f1..526634d819 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -137,6 +137,14 @@ static virClassPtr qemuDomainSaveCookieClass; static void qemuDomainLogContextDispose(void *obj); static void qemuDomainSaveCookieDispose(void *obj); + +static int +qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDefPtr disk, + virStorageSourcePtr src, + qemuDomainObjPrivatePtr priv, + virQEMUDriverConfigPtr cfg); + + static int qemuDomainOnceInit(void) { @@ -8638,6 +8646,10 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, if (qemuDomainPrepareDiskSourceData(disk, n, cfg, priv->qemuCaps) < 0) goto cleanup; + +if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV) && +qemuDomainPrepareStorageSourceBlockdev(disk, n, priv, cfg) < 0) +goto cleanup; } ret = 0; @@ -13085,6 +13097,61 @@ qemuDomainPrepareDiskSourceLegacy(virDomainDiskDefPtr disk, } +static int +qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDefPtr disk, + virStorageSourcePtr src, + qemuDomainObjPrivatePtr priv, + virQEMUDriverConfigPtr cfg) +{ +src->id = qemuDomainStorageIdNew(priv); + +if (virAsprintf(&src->nodestorage, "libvirt-%u-storage", src->id) < 0 || +virAsprintf(&src->nodeformat, "libvirt-%u-format", src->id) < 0) +return -1; + +if (qemuDomainValidateStorageSource(src, priv->qemuCaps) < 0) +return -1; + +if (qemuDomainPrepareDiskSourceData(disk, src, cfg, priv->qemuCaps) < 0) +return -1; + +if (qemuDomainSecretStorageSourcePrepare(priv, src, + src->nodestorage, + src->nodeformat) < 0) +return -1; + +if (qemuDomainPrepareStorageSourcePR(disk->src, priv, src->nodestorage) < 0) +return -1; + +if (qemuDomainPrepareStorageSourceTLS(disk->src, cfg, src->nodestorage, + priv->qemuCaps) < 0) +return -1; + +return 0; +} + + +static int +qemuDomainPrepareDiskSourceBlockdev(virDomainDiskDefPtr disk, +qemuDomainObjPrivatePtr priv, +virQEMUDriverConfigPtr cfg) +{ +qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); +virStorageSourcePtr n; + +if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON && +virAsprintf(&diskPriv->nodeCopyOnRead, "libvirt-CoR-%s", disk->dst) < 0) +return -1; + +for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) { +if (qemuDomainPrepareStorageSourceBlockdev(disk, n, priv, cfg) < 0) +return -1; +} + +return 0; +} + + int qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk, qemuDomainObjPrivatePtr priv, @@ -13092,8 +13159,13 @@ qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk, { qemuDomainPrepareDiskCachemode(disk); -if (qemuDomainPrepareDiskSourceLegacy(disk, priv, cfg) < 0) -return -1; +if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) { +if (qemuDomainPrepareDiskSourceBlockdev(disk, priv, cfg) < 0) +return -1; +} else { +if (qemuDomainPrepareDiskSourceLegacy(disk, priv, cfg) < 0) +return -1; +} return 0; } -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 21/55] qemu: domain: Add infrastructure to generate block node names
Node names for block objects in qemu need to be unique for an instance of the qemu process. Add a counter to generate objects sequentially and store it in the status XML so that we can restore it. The helpers added allow to create new node names and reset the counter after the VM process terminates. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 38 ++ src/qemu/qemu_domain.h | 6 ++ src/qemu/qemu_process.c | 3 +++ 3 files changed, 47 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index de056272e8..6103b86478 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2438,6 +2438,9 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, qemuDomainObjPrivateXMLFormatPR(buf, priv); +if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) +virBufferAsprintf(buf, "\n", priv->nodenameindex); + if (qemuDomainObjPrivateXMLFormatBlockjobs(buf, vm) < 0) return -1; @@ -2933,6 +2936,14 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, if (qemuDomainObjPrivateXMLParseBlockjobs(priv, ctxt) < 0) goto error; +qemuDomainStorageIdReset(priv); +if (virXPathULongLong("string(./nodename/@next)", ctxt, + &priv->nodenameindex) == -2) { +virReportError(VIR_ERR_XML_ERROR, "%s", + _("failed to parse node name index")); +goto error; +} + return 0; error: @@ -13161,3 +13172,30 @@ qemuDomainGetManagedPRSocketPath(qemuDomainObjPrivatePtr priv) return ret; } + + +/** + * qemuDomainStorageIdNew: + * @priv: qemu VM private data object. + * + * Generate a new unique id for a storage object. Useful for node name generation. + */ +unsigned int +qemuDomainStorageIdNew(qemuDomainObjPrivatePtr priv) +{ +return ++priv->nodenameindex; +} + + +/** + * qemuDomainStorageIdReset: + * @priv: qemu VM private data object. + * + * Resets the data for the node name generator. The node names need to be unique + * for a single instance, so can be reset on VM shutdown. + */ +void +qemuDomainStorageIdReset(qemuDomainObjPrivatePtr priv) +{ +priv->nodenameindex = 0; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 7b79d77257..5dcbdfbbe1 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -363,6 +363,9 @@ struct _qemuDomainObjPrivate { /* true if qemu-pr-helper process is running for the domain */ bool prDaemonRunning; + +/* counter for generating node names for qemu disks */ +unsigned long long nodenameindex; }; # define QEMU_DOMAIN_PRIVATE(vm) \ @@ -1063,4 +1066,7 @@ qemuDomainDiskCachemodeFlags(int cachemode, char * qemuDomainGetManagedPRSocketPath(qemuDomainObjPrivatePtr priv); +unsigned int qemuDomainStorageIdNew(qemuDomainObjPrivatePtr priv); +void qemuDomainStorageIdReset(qemuDomainObjPrivatePtr priv); + #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0a072c990d..334044f479 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7157,6 +7157,9 @@ void qemuProcessStop(virQEMUDriverPtr driver, /* clear all private data entries which are no longer needed */ qemuDomainObjPrivateDataClear(priv); +/* reset node name allocator */ +qemuDomainStorageIdReset(priv); + /* The "release" hook cleans up additional resources */ if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { char *xml = qemuDomainDefFormatXML(driver, vm->def, 0); -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 18/55] qemu: domain: Don't redetect backing chain when using -blockdev
We need to load the backing chain from the XML when using -blockdev. Signed-off-by: Peter Krempa --- src/qemu/qemu_process.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6b59dbc1c3..b01bb4fb5c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6027,8 +6027,10 @@ qemuProcessPrepareHostStorage(virQEMUDriverPtr driver, virDomainObjPtr vm, unsigned int flags) { +qemuDomainObjPrivatePtr priv = vm->privateData; size_t i; bool cold_boot = flags & VIR_QEMU_PROCESS_START_COLD; +bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); for (i = vm->def->ndisks; i > 0; i--) { size_t idx = i - 1; @@ -6037,7 +6039,9 @@ qemuProcessPrepareHostStorage(virQEMUDriverPtr driver, if (virStorageSourceIsEmpty(disk->src)) continue; -virStorageSourceBackingStoreClear(disk->src); +/* backing chain needs to be redetected if we aren't using blockdev */ +if (!blockdev) +virStorageSourceBackingStoreClear(disk->src); if (qemuDomainDetermineDiskChain(driver, vm, disk, true) >= 0) continue; @@ -7713,7 +7717,8 @@ qemuProcessReconnect(void *opaque) goto error; /* backing chains need to be refreshed only if they could change */ -if (priv->reconnectBlockjobs != VIR_TRISTATE_BOOL_NO) { +if (priv->reconnectBlockjobs != VIR_TRISTATE_BOOL_NO && +!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) { /* This should be the only place that calls * qemuDomainDetermineDiskChain with @report_broken == false * to guarantee best-effort domain reconnect */ -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/55] tests: qemu: Drop disk from hostdev-mdev tests
The disk is not necessary to test the mdevs. Signed-off-by: Peter Krempa --- tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml| 6 -- .../hostdev-mdev-display-spice-egl-headless.x86_64-latest.args | 2 -- tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.xml | 6 -- .../hostdev-mdev-display-spice-opengl.x86_64-latest.args| 2 -- tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml| 6 -- .../hostdev-mdev-display-vnc-egl-headless.x86_64-latest.args| 2 -- tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.xml| 6 -- tests/qemuxml2argvdata/hostdev-mdev-display-vnc.x86_64-latest.args | 2 -- tests/qemuxml2argvdata/hostdev-mdev-display-vnc.xml | 6 -- tests/qemuxml2argvdata/hostdev-mdev-display.xml | 6 -- tests/qemuxml2argvdata/hostdev-mdev-invalid-target-address.xml | 5 - tests/qemuxml2argvdata/hostdev-mdev-precreated.args | 2 -- tests/qemuxml2argvdata/hostdev-mdev-precreated.xml | 6 -- tests/qemuxml2argvdata/hostdev-mdev-src-address-invalid.xml | 6 -- tests/qemuxml2xmloutdata/hostdev-mdev-display.xml | 6 -- tests/qemuxml2xmloutdata/hostdev-mdev-precreated.xml| 6 -- 16 files changed, 75 deletions(-) diff --git a/tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml b/tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml index ea559a6444..55b60ba133 100644 --- a/tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml +++ b/tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml @@ -14,12 +14,6 @@ destroy /usr/bin/qemu-system-i686 - - - - - - diff --git a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.x86_64-latest.args b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.x86_64-latest.args index 0ac90c81d2..b84869264e 100644 --- a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.x86_64-latest.args +++ b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.x86_64-latest.args @@ -23,8 +23,6 @@ file=/tmp/lib/domain--1-QEMUGuest2/master-key.aes \ -no-acpi \ -boot strict=on \ -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ --drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \ --device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ -spice port=0,seamless-migration=on \ -display egl-headless \ -device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,\ diff --git a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.xml b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.xml index c8f10c2f3a..3a686ad2bf 100644 --- a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.xml +++ b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.xml @@ -14,12 +14,6 @@ destroy /usr/bin/qemu-system-i686 - - - - - - diff --git a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.x86_64-latest.args b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.x86_64-latest.args index 1fd9fdaa16..80c56abfb9 100644 --- a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.x86_64-latest.args +++ b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.x86_64-latest.args @@ -23,8 +23,6 @@ file=/tmp/lib/domain--1-QEMUGuest2/master-key.aes \ -no-acpi \ -boot strict=on \ -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ --drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \ --device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ -spice port=0,gl=on,rendernode=/dev/dri/foo,seamless-migration=on \ -device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,\ vram64_size_mb=0,vgamem_mb=16,max_outputs=1,bus=pci.0,addr=0x2 \ diff --git a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml index 18c9817608..a632e58a41 100644 --- a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml +++ b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml @@ -14,12 +14,6 @@ destroy /usr/bin/qemu-system-i686 - - - - - - diff --git a/tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.x86_64-latest.args b/tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.x86_64-latest.args index cdf545d0e0..91708d7663 100644 --- a/tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.x86_64-latest.args +++ b/tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.x86_64-latest.args @@ -23,8 +23,6 @@ file=/tmp/lib/domain--1-QEMUGuest2/master-key.aes \ -no-acpi \ -boot strict=on \ -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ --drive file=
[libvirt] [PATCH 10/55] qemu: monitor: Add 'nodename' argument for 'block_resize'
Allow referring to individual node name to rezise. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_monitor.c | 12 ++-- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 4 +++- src/qemu/qemu_monitor_json.h | 3 ++- tests/qemumonitorjsontest.c | 2 +- 6 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 15ec4931eb..9ee8d33f8d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11005,7 +11005,7 @@ qemuDomainBlockResize(virDomainPtr dom, goto endjob; qemuDomainObjEnterMonitor(driver, vm); -if (qemuMonitorBlockResize(priv->mon, device, size) < 0) { +if (qemuMonitorBlockResize(priv->mon, device, NULL, size) < 0) { ignore_value(qemuDomainObjExitMonitor(driver, vm)); goto endjob; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 758942ffcb..4dcbd69dce 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2312,13 +2312,21 @@ qemuMonitorBlockStatsUpdateCapacity(qemuMonitorPtr mon, int qemuMonitorBlockResize(qemuMonitorPtr mon, const char *device, + const char *nodename, unsigned long long size) { -VIR_DEBUG("device=%s size=%llu", device, size); +VIR_DEBUG("device=%s nodename=%s size=%llu", + NULLSTR(device), NULLSTR(nodename), size); QEMU_CHECK_MONITOR(mon); -return qemuMonitorJSONBlockResize(mon, device, size); +if ((!device && !nodename) || (device && nodename)) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("exactly one of 'device' and 'nodename' need to be specified")); +return -1; +} + +return qemuMonitorJSONBlockResize(mon, device, nodename, size); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 60418422e9..f8f6969ddb 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -596,7 +596,8 @@ int qemuMonitorBlockStatsUpdateCapacity(qemuMonitorPtr mon, ATTRIBUTE_NONNULL(2); int qemuMonitorBlockResize(qemuMonitorPtr mon, - const char *dev_name, + const char *device, + const char *nodename, unsigned long long size); int qemuMonitorSetVNCPassword(qemuMonitorPtr mon, const char *password); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index e43447dc53..de3fcd83d8 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2542,6 +2542,7 @@ qemuMonitorJSONBlockStatsUpdateCapacity(qemuMonitorPtr mon, int qemuMonitorJSONBlockResize(qemuMonitorPtr mon, const char *device, + const char *nodename, unsigned long long size) { int ret = -1; @@ -2549,7 +2550,8 @@ int qemuMonitorJSONBlockResize(qemuMonitorPtr mon, virJSONValuePtr reply = NULL; cmd = qemuMonitorJSONMakeCommand("block_resize", - "s:device", device, + "S:device", device, + "S:node-name", nodename, "U:size", size, NULL); if (!cmd) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 4d75e04183..19aebef5fb 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -94,7 +94,8 @@ int qemuMonitorJSONBlockStatsUpdateCapacity(qemuMonitorPtr mon, virHashTablePtr stats, bool backingChain); int qemuMonitorJSONBlockResize(qemuMonitorPtr mon, - const char *devce, + const char *device, + const char *nodename, unsigned long long size); int qemuMonitorJSONSetVNCPassword(qemuMonitorPtr mon, diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index fd54031e40..b048c40d7d 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1320,7 +1320,7 @@ cleanup: \ } GEN_TEST_FUNC(qemuMonitorJSONSetLink, "vnet0", VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN) -GEN_TEST_FUNC(qemuMonitorJSONBlockResize, "vda", 123456) +GEN_TEST_FUNC(qemuMonitorJSONBlockResize, "vda", "asdf", 123456) GEN_TEST_FUNC(qemuMonitorJSONSetVNCPassword, "secret_password") GEN_TEST_FUNC(qemuMonitorJSONSetPassword, "spice", "secret_password", "disconnect") GEN_TEST_FUNC(qemuMonitorJSONExpirePassword, "spice", "123456") -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/55] qemu: monitor: Allow using 'qdev' instead of 'device' for getting disk throttling
The 'device' field reported by 'query-block' is empty when -blockdev is used. Add an argument which will allow matching disk by using the qdev id so we can use this code with -blockdev. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_monitor.c | 8 +--- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 21 ++--- src/qemu/qemu_monitor_json.h | 3 ++- tests/qemumonitorjsontest.c | 2 +- 6 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 99fd3bebf5..15ec4931eb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18617,7 +18617,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, if (!(device = qemuAliasDiskDriveFromDisk(disk))) goto endjob; qemuDomainObjEnterMonitor(driver, vm); -ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, &reply); +ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, NULL, &reply); if (qemuDomainObjExitMonitor(driver, vm) < 0) goto endjob; if (ret < 0) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 2902c3b246..758942ffcb 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3465,14 +3465,16 @@ qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon, int qemuMonitorGetBlockIoThrottle(qemuMonitorPtr mon, - const char *device, + const char *drivealias, + const char *qdevid, virDomainBlockIoTuneInfoPtr reply) { -VIR_DEBUG("device=%p, reply=%p", device, reply); +VIR_DEBUG("drivealias=%s, qdevid=%s, reply=%p", + NULLSTR(drivealias), NULLSTR(qdevid), reply); QEMU_CHECK_MONITOR(mon); -return qemuMonitorJSONGetBlockIoThrottle(mon, device, reply); +return qemuMonitorJSONGetBlockIoThrottle(mon, drivealias, qdevid, reply); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 16fc75819f..60418422e9 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -940,7 +940,8 @@ int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon, bool supportMaxLengthOptions); int qemuMonitorGetBlockIoThrottle(qemuMonitorPtr mon, - const char *device, + const char *drivealias, + const char *qdevid, virDomainBlockIoTuneInfoPtr reply); int qemuMonitorSystemWakeup(qemuMonitorPtr mon); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3a1dfb8563..e43447dc53 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4818,7 +4818,8 @@ int qemuMonitorJSONOpenGraphics(qemuMonitorPtr mon, } static int qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr io_throttle, - const char *device, + const char *drivealias, + const char *qdevid, virDomainBlockIoTuneInfoPtr reply) { int ret = -1; @@ -4828,7 +4829,8 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr io_throttle, for (i = 0; i < virJSONValueArraySize(io_throttle); i++) { virJSONValuePtr temp_dev = virJSONValueArrayGet(io_throttle, i); virJSONValuePtr inserted; -const char *current_dev; +const char *current_drive; +const char *current_qdev; if (!temp_dev || virJSONValueGetType(temp_dev) != VIR_JSON_TYPE_OBJECT) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -4837,14 +4839,18 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr io_throttle, goto cleanup; } -if (!(current_dev = virJSONValueObjectGetString(temp_dev, "device"))) { +current_qdev = virJSONValueObjectGetString(temp_dev, "qdev"); +current_drive = virJSONValueObjectGetString(temp_dev, "device"); + +if (!current_drive && !current_qdev) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("block_io_throttle device entry " "was not in expected format")); goto cleanup; } -if (STRNEQ(current_dev, device)) +if ((drivealias && STRNEQ(current_drive, drivealias)) || +(qdevid && STRNEQ(current_qdev, qdevid))) continue; found = true; @@ -4885,7 +4891,7 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr io_throttle, if (!found) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find throttling info for device '%s'"), - device); + drivealias ? drivealias : qdevid); goto cleanup; } ret = 0; @@ -4996,7 +5002,8 @@ int qemuMonitor
[libvirt] [PATCH 54/55] qemu: driver: Prepare qemuDomainGetStatsBlock (bulk disk stats) for -blockdev
Add code paths which call into the new functions to gather the data on a per-node-name basis and tweak the aliases used for extracting the data. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 54 +++--- 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7309ce92e4..2a5a7d70ed 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20257,20 +20257,34 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, bool visitBacking, virQEMUDriverPtr driver, virQEMUDriverConfigPtr cfg, - virDomainObjPtr dom) + virDomainObjPtr dom, + bool blockdev) { char *alias = NULL; virStorageSourcePtr n; +const char *frontendalias; +const char *backendalias; +const char *backendstoragealias; int ret = -1; for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) { -/* alias may be NULL if the VM is not running */ -if (disk->info.alias && -!(alias = qemuDomainStorageAlias(disk->info.alias, n->id))) -goto cleanup; +if (blockdev) { +frontendalias = QEMU_DOMAIN_DISK_PRIVATE(disk)->backendQomName; +backendalias = n->nodeformat; +backendstoragealias = n->nodestorage; +} else { +/* alias may be NULL if the VM is not running */ +if (disk->info.alias && +!(alias = qemuDomainStorageAlias(disk->info.alias, n->id))) +goto cleanup; + +qemuDomainGetStatsOneBlockRefreshNamed(n, alias, stats, nodestats); -qemuDomainGetStatsOneBlockRefreshNamed(n, alias, stats, nodestats); +frontendalias = alias; +backendalias = alias; +backendstoragealias = alias; +} if (qemuDomainGetStatsBlockExportHeader(disk, n, *recordnr, records, nrecords) < 0) @@ -20278,17 +20292,17 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, /* The following stats make sense only for the frontend device */ if (n == disk->src) { -if (qemuDomainGetStatsBlockExportFrontend(alias, stats, *recordnr, +if (qemuDomainGetStatsBlockExportFrontend(frontendalias, stats, *recordnr, records, nrecords) < 0) goto cleanup; } if (qemuDomainGetStatsOneBlock(driver, cfg, dom, records, nrecords, - alias, n, *recordnr, + backendalias, n, *recordnr, stats) < 0) goto cleanup; -if (qemuDomainGetStatsBlockExportBackendStorage(alias, +if (qemuDomainGetStatsBlockExportBackendStorage(backendstoragealias, stats, *recordnr, records, nrecords) < 0) goto cleanup; @@ -20323,19 +20337,26 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, virJSONValuePtr nodedata = NULL; qemuDomainObjPrivatePtr priv = dom->privateData; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); +bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); bool fetchnodedata = virQEMUCapsGet(priv->qemuCaps, -QEMU_CAPS_QUERY_NAMED_BLOCK_NODES); +QEMU_CAPS_QUERY_NAMED_BLOCK_NODES) && !blockdev; int count_index = -1; size_t visited = 0; bool visitBacking = !!(privflags & QEMU_DOMAIN_STATS_BACKING); if (HAVE_JOB(privflags) && virDomainObjIsActive(dom)) { qemuDomainObjEnterMonitor(driver, dom); -rc = qemuMonitorGetAllBlockStatsInfo(priv->mon, &stats, - visitBacking); -if (rc >= 0) -ignore_value(qemuMonitorBlockStatsUpdateCapacity(priv->mon, stats, - visitBacking)); + +rc = qemuMonitorGetAllBlockStatsInfo(priv->mon, &stats, visitBacking); + +if (blockdev) { +if (rc >= 0) +rc = qemuMonitorBlockStatsUpdateCapacityBlockdev(priv->mon, stats); +} else { +if (rc >= 0) +ignore_value(qemuMonitorBlockStatsUpdateCapacity(priv->mon, stats, + visitBacking)); +} if (fetchnodedata) nodedata = qemuMonitorQueryNamedBlockNodes(priv->mon); @@ -20361,7 +20382,8 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, for (i = 0; i < dom->def->nd
[libvirt] [PATCH 42/55] qemu: monitor: Add APIs for refreshing disk capacity when using -blockdev
Disk image size data are not contained in the reply of query-blockstats but need to be gathered from query-block. For use with -blockdev we really need to call 'query-named-block-nodes' and process it to retrieve the correct data. This patch introduces qemuMonitorBlockStatsUpdateCapacityBlockdev which updates the capacity data by nodename rather than device name. Signed-off-by: Peter Krempa --- src/qemu/qemu_monitor.c | 11 +++ src/qemu/qemu_monitor.h | 4 src/qemu/qemu_monitor_json.c | 46 src/qemu/qemu_monitor_json.h | 3 +++ 4 files changed, 64 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index d665f33d81..ed6a53e3a5 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2309,6 +2309,17 @@ qemuMonitorBlockStatsUpdateCapacity(qemuMonitorPtr mon, } +int +qemuMonitorBlockStatsUpdateCapacityBlockdev(qemuMonitorPtr mon, +virHashTablePtr stats) +{ +VIR_DEBUG("stats=%p", stats); + +QEMU_CHECK_MONITOR(mon); + +return qemuMonitorJSONBlockStatsUpdateCapacityBlockdev(mon, stats); +} + int qemuMonitorBlockResize(qemuMonitorPtr mon, const char *device, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 0734588f66..3e4822c6ec 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -595,6 +595,10 @@ int qemuMonitorBlockStatsUpdateCapacity(qemuMonitorPtr mon, bool backingChain) ATTRIBUTE_NONNULL(2); +int qemuMonitorBlockStatsUpdateCapacityBlockdev(qemuMonitorPtr mon, +virHashTablePtr stats) +ATTRIBUTE_NONNULL(2); + int qemuMonitorBlockResize(qemuMonitorPtr mon, const char *device, const char *nodename, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 24e3d61699..51d6ee2917 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2585,6 +2585,52 @@ qemuMonitorJSONBlockStatsUpdateCapacity(qemuMonitorPtr mon, } +static int +qemuMonitorJSONBlockStatsUpdateCapacityBlockdevWorker(size_t pos ATTRIBUTE_UNUSED, + virJSONValuePtr val, + void *opaque) +{ +virHashTablePtr stats = opaque; +virJSONValuePtr image; +const char *nodename; + +if (!(nodename = virJSONValueObjectGetString(val, "node-name")) || +!(image = virJSONValueObjectGetObject(val, "image"))) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-named-block-nodes entry was not in expected format")); +return -1; +} + +if (qemuMonitorJSONBlockStatsUpdateCapacityData(image, nodename, stats) < 0) +return -1; + +return 1; /* we don't want to steal the value from the JSON array */ +} + + +int +qemuMonitorJSONBlockStatsUpdateCapacityBlockdev(qemuMonitorPtr mon, +virHashTablePtr stats) +{ +virJSONValuePtr nodes; +int ret = -1; + +if (!(nodes = qemuMonitorJSONQueryNamedBlockNodes(mon))) +return -1; + +if (virJSONValueArrayForeachSteal(nodes, + qemuMonitorJSONBlockStatsUpdateCapacityBlockdevWorker, + stats) < 0) +goto cleanup; + +ret = 0; + + cleanup: +virJSONValueFree(nodes); +return ret; +} + + int qemuMonitorJSONBlockResize(qemuMonitorPtr mon, const char *device, const char *nodename, diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 3c5839db38..51214e3797 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -93,6 +93,9 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, int qemuMonitorJSONBlockStatsUpdateCapacity(qemuMonitorPtr mon, virHashTablePtr stats, bool backingChain); +int qemuMonitorJSONBlockStatsUpdateCapacityBlockdev(qemuMonitorPtr mon, +virHashTablePtr stats); + int qemuMonitorJSONBlockResize(qemuMonitorPtr mon, const char *device, const char *nodename, -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 41/55] qemu: monitor: Retrieve blockstats also by qdev and node-names
For use with -blockdev we need to be able to retrieve the stats by 'qdev' for the frontend device stats since 'device' will be NULL. Additionally so that we can report the highest written offset we need to also be able to access them by node-name for backing chain purposes. In cases when 'device' is empty it does not make sense to gather them. Allow arranging the stats simultaneously in all the above dimensions. Signed-off-by: Peter Krempa --- src/qemu/qemu_monitor_json.c | 55 1 file changed, 50 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 105f6f78ac..24e3d61699 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2324,6 +2324,28 @@ qemuMonitorJSONBlockStatsCollectData(virJSONValuePtr dev, } +static int +qemuMonitorJSONAddOneBlockStatsInfo(qemuBlockStatsPtr bstats, +const char *name, +virHashTablePtr stats) +{ +qemuBlockStatsPtr copy = NULL; + +if (VIR_ALLOC(copy) < 0) +return -1; + +if (bstats) +*copy = *bstats; + +if (virHashAddEntry(stats, name, copy) < 0) { +VIR_FREE(copy); +return -1; +} + +return 0; +} + + static int qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev, const char *dev_name, @@ -2334,18 +2356,38 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev, qemuBlockStatsPtr bstats = NULL; int ret = -1; int nstats = 0; -char *entry_name = qemuDomainStorageAlias(dev_name, depth); +const char *qdevname = NULL; +const char *nodename = NULL; +char *devicename = NULL; virJSONValuePtr backing; -if (!entry_name) +if (dev_name && +!(devicename = qemuDomainStorageAlias(dev_name, depth))) goto cleanup; +qdevname = virJSONValueObjectGetString(dev, "qdev"); +nodename = virJSONValueObjectGetString(dev, "node-name"); + +if (!devicename && !qdevname && !nodename) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("blockstats device entry was not in expected format")); +goto cleanup; +} + if (!(bstats = qemuMonitorJSONBlockStatsCollectData(dev, &nstats))) goto cleanup; -if (virHashAddEntry(hash, entry_name, bstats) < 0) +if (devicename && +qemuMonitorJSONAddOneBlockStatsInfo(bstats, devicename, hash) < 0) +goto cleanup; + +if (qdevname && +qemuMonitorJSONAddOneBlockStatsInfo(bstats, qdevname, hash) < 0) +goto cleanup; + +if (nodename && +qemuMonitorJSONAddOneBlockStatsInfo(bstats, nodename, hash) < 0) goto cleanup; -bstats = NULL; if (backingChain && (backing = virJSONValueObjectGetObject(dev, "backing")) && @@ -2356,7 +2398,7 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev, ret = nstats; cleanup: VIR_FREE(bstats); -VIR_FREE(entry_name); +VIR_FREE(devicename); return ret; } @@ -2418,6 +2460,9 @@ qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, goto cleanup; } +if (*dev_name == '\0') +dev_name = NULL; + rc = qemuMonitorJSONGetOneBlockStatsInfo(dev, dev_name, 0, hash, backingChain); -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 52/55] qemu: Use QOM path with query-block when using -blockdev
Switch to using the QOM/qdev handles in all calls to qemuMonitorGetBlockInfo when using -blockdev. The callers also need to make sure to use the correct handle afterwards to extract the data. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 9 - src/qemu/qemu_process.c | 7 ++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 63c1a85d5a..6ee826029b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18738,6 +18738,7 @@ qemuDomainGetDiskErrors(virDomainPtr dom, virDomainObjPtr vm = NULL; qemuDomainObjPrivatePtr priv; virHashTablePtr table = NULL; +bool blockdev = false; int ret = -1; size_t i; int n = 0; @@ -18748,6 +18749,7 @@ qemuDomainGetDiskErrors(virDomainPtr dom, goto cleanup; priv = vm->privateData; +blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); if (virDomainGetDiskErrorsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -18773,8 +18775,13 @@ qemuDomainGetDiskErrors(virDomainPtr dom, for (i = n = 0; i < vm->def->ndisks; i++) { struct qemuDomainDiskInfo *info; virDomainDiskDefPtr disk = vm->def->disks[i]; +qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); +const char *entryname = disk->info.alias; + +if (blockdev) +entryname = diskPriv->backendQomName; -if ((info = virHashLookup(table, disk->info.alias)) && +if ((info = virHashLookup(table, entryname)) && info->io_status != VIR_DOMAIN_DISK_ERROR_NONE) { if (n == nerrors) break; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 2183127133..fceaf95fab 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7561,6 +7561,7 @@ qemuProcessRefreshDisks(virQEMUDriverPtr driver, qemuDomainAsyncJob asyncJob) { qemuDomainObjPrivatePtr priv = vm->privateData; +bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); virHashTablePtr table = NULL; int ret = -1; size_t i; @@ -7578,8 +7579,12 @@ qemuProcessRefreshDisks(virQEMUDriverPtr driver, virDomainDiskDefPtr disk = vm->def->disks[i]; qemuDomainDiskPrivatePtr diskpriv = QEMU_DOMAIN_DISK_PRIVATE(disk); struct qemuDomainDiskInfo *info; +const char *entryname = disk->info.alias; + +if (blockdev) +entryname = diskpriv->backendQomName; -if (!(info = virHashLookup(table, disk->info.alias))) +if (!(info = virHashLookup(table, entryname))) continue; if (info->removable) { -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 51/55] qemu: monitor: Report data also for 'qdev' entry in qemuMonitorJSONGetBlockInfo
With -blockdev qemu will not report any useful "device" for the data returned by 'query-block'. We need to start using the 'qdev' field to do so. This patch adds data for the 'qdev' field into the returned data structure. Signed-off-by: Peter Krempa --- src/qemu/qemu_monitor_json.c | 69 +++- 1 file changed, 55 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 75f40716ea..0579554016 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2201,6 +2201,38 @@ qemuMonitorJSONGetBlockDevDevice(virJSONValuePtr dev) } +static int +qemuMonitorJSONBlockInfoAdd(virHashTablePtr table, +struct qemuDomainDiskInfo *info, +const char *entryname) +{ +struct qemuDomainDiskInfo *tmp = NULL; +int ret = -1; + +if (VIR_ALLOC(tmp) < 0) +goto cleanup; + +*tmp = *info; +tmp->nodename = NULL; + +if (info->nodename && +VIR_STRDUP(tmp->nodename, info->nodename) < 0) +goto cleanup; + +if (virHashAddEntry(table, entryname, tmp) < 0) +goto cleanup; + +tmp = NULL; +ret = 0; + + cleanup: +if (tmp) +VIR_FREE(tmp->nodename); +VIR_FREE(tmp); +return ret; +} + + int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, virHashTablePtr table) { @@ -2215,10 +2247,10 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, for (i = 0; i < virJSONValueArraySize(devices); i++) { virJSONValuePtr dev; virJSONValuePtr image; -struct qemuDomainDiskInfo *info; +struct qemuDomainDiskInfo info = { false }; const char *thisdev; const char *status; -const char *nodename; +const char *qdev; if (!(dev = qemuMonitorJSONGetBlockDev(devices, i))) goto cleanup; @@ -2227,16 +2259,18 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, goto cleanup; thisdev = qemuAliasDiskDriveSkipPrefix(thisdev); +qdev = virJSONValueObjectGetString(dev, "qdev"); -if (VIR_ALLOC(info) < 0) -goto cleanup; +if (*thisdev == '\0') +thisdev = NULL; -if (virHashAddEntry(table, thisdev, info) < 0) { -VIR_FREE(info); +if (!qdev && !thisdev) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-block device entry was not in expected format")); goto cleanup; } -if (virJSONValueObjectGetBoolean(dev, "removable", &info->removable) < 0) { +if (virJSONValueObjectGetBoolean(dev, "removable", &info.removable) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot read %s value"), "removable"); @@ -2244,23 +2278,30 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, } /* 'tray_open' is present only if the device has a tray */ -if (virJSONValueObjectGetBoolean(dev, "tray_open", &info->tray_open) == 0) -info->tray = true; +if (virJSONValueObjectGetBoolean(dev, "tray_open", &info.tray_open) == 0) +info.tray = true; /* presence of 'inserted' notifies that a medium is in the device */ if ((image = virJSONValueObjectGetObject(dev, "inserted"))) { -if ((nodename = virJSONValueObjectGetString(image, "node-name"))) -ignore_value(VIR_STRDUP(info->nodename, nodename)); +info.nodename = (char *) virJSONValueObjectGetString(image, "node-name"); } else { -info->empty = true; +info.empty = true; } /* Missing io-status indicates no error */ if ((status = virJSONValueObjectGetString(dev, "io-status"))) { -info->io_status = qemuMonitorBlockIOStatusToError(status); -if (info->io_status < 0) +info.io_status = qemuMonitorBlockIOStatusToError(status); +if (info.io_status < 0) goto cleanup; } + +if (thisdev && +qemuMonitorJSONBlockInfoAdd(table, &info, thisdev) < 0) +goto cleanup; + +if (qdev && +qemuMonitorJSONBlockInfoAdd(table, &info, qdev) < 0) +goto cleanup; } ret = 0; -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 46/55] qemu: Extract exporting of the header for block stats
Split out the header so that the loop can be refactored later. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 38 -- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ddd991546a..4e9be6ed3d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20085,7 +20085,6 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver, virDomainObjPtr dom, virDomainStatsRecordPtr record, int *maxparams, - const char *diskdst, const char *entryname, virStorageSourcePtr src, size_t block_idx, @@ -20094,14 +20093,6 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver, qemuBlockStats *entry; int ret = -1; -QEMU_ADD_NAME_PARAM(record, maxparams, "block", "name", block_idx, diskdst); - -if (virStorageSourceIsLocalStorage(src) && src->path) -QEMU_ADD_NAME_PARAM(record, maxparams, "block", "path", -block_idx, src->path); -if (src->id) -QEMU_ADD_BLOCK_PARAM_UI(record, maxparams, block_idx, "backingIndex", -src->id); /* the VM is offline so we have to go and load the stast from the disk by * ourselves */ @@ -20164,6 +20155,29 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver, } +static int +qemuDomainGetStatsBlockExportHeader(virDomainDiskDefPtr disk, +virStorageSourcePtr src, +size_t recordnr, +virDomainStatsRecordPtr records, +int *nrecords) +{ +int ret = -1; + +QEMU_ADD_NAME_PARAM(records, nrecords, "block", "name", recordnr, disk->dst); + +if (virStorageSourceIsLocalStorage(src) && src->path) +QEMU_ADD_NAME_PARAM(records, nrecords, "block", "path", recordnr, src->path); +if (src->id) +QEMU_ADD_BLOCK_PARAM_UI(records, nrecords, recordnr, "backingIndex", +src->id); + +ret = 0; + cleanup: +return ret; +} + + static int qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, virHashTablePtr stats, @@ -20191,8 +20205,12 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, qemuDomainGetStatsOneBlockRefreshNamed(src, alias, stats, nodestats); +if (qemuDomainGetStatsBlockExportHeader(disk, src, *recordnr, +records, nrecords) < 0) +goto cleanup; + if (qemuDomainGetStatsOneBlock(driver, cfg, dom, records, nrecords, - disk->dst, alias, src, *recordnr, + alias, src, *recordnr, stats) < 0) goto cleanup; -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 49/55] qemu: Report frontend stats only for the frontend entry
When reporting stats for the backing chain some of them make sense only for the topmost entry as they are actually tied to the frontend device. We unfortunately can't change that fact, but we can stop reporting all zero stats for the backing chain members where they don't make any sense. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d0a680629e..ee401eb702 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20223,9 +20223,12 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, records, nrecords) < 0) goto cleanup; -if (qemuDomainGetStatsBlockExportFrontend(alias, stats, *recordnr, - records, nrecords) < 0) -goto cleanup; +/* The following stats make sense only for the frontend device */ +if (n == disk->src) { +if (qemuDomainGetStatsBlockExportFrontend(alias, stats, *recordnr, + records, nrecords) < 0) +goto cleanup; +} if (qemuDomainGetStatsOneBlock(driver, cfg, dom, records, nrecords, alias, n, *recordnr, -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 47/55] qemu: Export stats relevant for the frontend separately
While we report the read and written byte stats for every single layer of the backing chain, qemu in fact reports them only for the frontend. Split out the relevant stats into a separate function so that we can later fix this bug and stop reporting it for backing chain entries where they don't make sense. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 54 ++ 1 file changed, 37 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4e9be6ed3d..d8a202fa47 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20110,23 +20110,6 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver, goto cleanup; } -QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx, -"rd.reqs", entry->rd_req); -QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx, -"rd.bytes", entry->rd_bytes); -QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx, -"rd.times", entry->rd_total_times); -QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx, -"wr.reqs", entry->wr_req); -QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx, -"wr.bytes", entry->wr_bytes); -QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx, -"wr.times", entry->wr_total_times); -QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx, -"fl.reqs", entry->flush_req); -QEMU_ADD_BLOCK_PARAM_LL(record, maxparams, block_idx, -"fl.times", entry->flush_total_times); - QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx, "allocation", entry->wr_highest_offset); @@ -20155,6 +20138,39 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver, } +static int +qemuDomainGetStatsBlockExportFrontend(const char *frontendname, + virHashTablePtr stats, + size_t recordnr, + virDomainStatsRecordPtr records, + int *nrecords) +{ +qemuBlockStats *entry; +int ret = -1; + +/* In case where qemu didn't provide the stats we stop here rather than + * trying to refresh the stats from the disk. Inability to provide stats is + * usually caused by blocked storage so this would make libvirtd hang */ +if (!stats || !frontendname || !(entry = virHashLookup(stats, frontendname))) { +ret = 0; +goto cleanup; +} + +QEMU_ADD_BLOCK_PARAM_LL(records, nrecords, recordnr, "rd.reqs", entry->rd_req); +QEMU_ADD_BLOCK_PARAM_LL(records, nrecords, recordnr, "rd.bytes", entry->rd_bytes); +QEMU_ADD_BLOCK_PARAM_LL(records, nrecords, recordnr, "rd.times", entry->rd_total_times); +QEMU_ADD_BLOCK_PARAM_LL(records, nrecords, recordnr, "wr.reqs", entry->wr_req); +QEMU_ADD_BLOCK_PARAM_LL(records, nrecords, recordnr, "wr.bytes", entry->wr_bytes); +QEMU_ADD_BLOCK_PARAM_LL(records, nrecords, recordnr, "wr.times", entry->wr_total_times); +QEMU_ADD_BLOCK_PARAM_LL(records, nrecords, recordnr, "fl.reqs", entry->flush_req); +QEMU_ADD_BLOCK_PARAM_LL(records, nrecords, recordnr, "fl.times", entry->flush_total_times); + +ret = 0; + cleanup: +return ret; +} + + static int qemuDomainGetStatsBlockExportHeader(virDomainDiskDefPtr disk, virStorageSourcePtr src, @@ -20209,6 +20225,10 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, records, nrecords) < 0) goto cleanup; +if (qemuDomainGetStatsBlockExportFrontend(alias, stats, *recordnr, + records, nrecords) < 0) +goto cleanup; + if (qemuDomainGetStatsOneBlock(driver, cfg, dom, records, nrecords, alias, src, *recordnr, stats) < 0) -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 44/55] qemu: monitor: Extract 'write-threshold' automatically for -blockdev
In cases when -blockdev is used we need to use 'query-named-block-nodes' instead of 'query-block'. This means that we can extract the write-threshold variable right away. To keep compatibility with old VMs modify the code which was extracting the value previously so that it updates the stats structure and a single code path then can be used to extract the data. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 57 ++-- src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 16 ++--- 3 files changed, 50 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ed0c563c85..b0ac5e536b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20043,29 +20043,39 @@ qemuDomainGetStatsOneBlockFallback(virQEMUDriverPtr driver, } -static int -qemuDomainGetStatsOneBlockNode(virDomainStatsRecordPtr record, - int *maxparams, - virStorageSourcePtr src, - size_t block_idx, - virHashTablePtr nodedata) +/** + * qemuDomainGetStatsOneBlockRefreshNamed: + * @src: disk source structure + * @alias: disk alias + * @stats: hash table containing stats for all disks + * @nodedata: reply containin 'query-named-block-nodes' data + * + * Refresh disk block stats data (qemuBlockStatsPtr) which are present only + * in the reply of 'query-named-block-nodes' in cases when the data was gathered + * by using qem-block originally. + */ +static void +qemuDomainGetStatsOneBlockRefreshNamed(virStorageSourcePtr src, + const char *alias, + virHashTablePtr stats, + virHashTablePtr nodedata) { +qemuBlockStatsPtr entry; + virJSONValuePtr data; unsigned long long tmp; -int ret = -1; -if (src->nodestorage && -(data = virHashLookup(nodedata, src->nodestorage))) { -if (virJSONValueObjectGetNumberUlong(data, "write_threshold", &tmp) == 0 && -tmp > 0) -QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx, - "threshold", tmp); -} +if (!nodedata || !src->nodestorage) +return; -ret = 0; +if (!(entry = virHashLookup(stats, alias))) +return; - cleanup: -return ret; +if (!(data = virHashLookup(nodedata, src->nodestorage))) +return; + +if (virJSONValueObjectGetNumberUlong(data, "write_threshold", &tmp) == 0) +entry->write_threshold = tmp; } @@ -20079,8 +20089,7 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver, const char *entryname, virStorageSourcePtr src, size_t block_idx, - virHashTablePtr stats, - virHashTablePtr nodedata) + virHashTablePtr stats) { qemuBlockStats *entry; int ret = -1; @@ -20145,9 +20154,9 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver, } } -if (qemuDomainGetStatsOneBlockNode(record, maxparams, src, block_idx, - nodedata) < 0) -goto cleanup; +if (entry->write_threshold) +QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx, "threshold", + entry->write_threshold); ret = 0; cleanup: @@ -20218,9 +20227,11 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver, !(alias = qemuDomainStorageAlias(disk->info.alias, src->id))) goto cleanup; +qemuDomainGetStatsOneBlockRefreshNamed(src, alias, stats, nodestats); + if (qemuDomainGetStatsOneBlock(driver, cfg, dom, record, maxparams, disk->dst, alias, src, visited, - stats, nodestats) < 0) + stats) < 0) goto cleanup; VIR_FREE(alias); diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 3e4822c6ec..431848856a 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -583,6 +583,9 @@ struct _qemuBlockStats { * if wr_highest_offset_valid is true */ unsigned long long wr_highest_offset; bool wr_highest_offset_valid; + +/* write_threshold is valid only if it's non-zero, conforming to qemu semantics */ +unsigned long long write_threshold; }; int qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 51d6ee2917..75f40716ea 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2484,7 +2484,8 @@ qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, static int qemuMonitorJSONBlockStatsUpdateCapacityData(virJSONValuePt
[libvirt] [PATCH 48/55] qemu: Refactor control flow in qemuDomainGetStatsBlockExportDisk
Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d8a202fa47..d0a680629e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20208,20 +20208,18 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, { char *alias = NULL; -virStorageSourcePtr src = disk->src; +virStorageSourcePtr n; int ret = -1; -while (virStorageSourceIsBacking(src) && - (src == disk->src || visitBacking)) { - +for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) { /* alias may be NULL if the VM is not running */ if (disk->info.alias && -!(alias = qemuDomainStorageAlias(disk->info.alias, src->id))) +!(alias = qemuDomainStorageAlias(disk->info.alias, n->id))) goto cleanup; -qemuDomainGetStatsOneBlockRefreshNamed(src, alias, stats, nodestats); +qemuDomainGetStatsOneBlockRefreshNamed(n, alias, stats, nodestats); -if (qemuDomainGetStatsBlockExportHeader(disk, src, *recordnr, +if (qemuDomainGetStatsBlockExportHeader(disk, n, *recordnr, records, nrecords) < 0) goto cleanup; @@ -20230,13 +20228,15 @@ qemuDomainGetStatsBlockExportDisk(virDomainDiskDefPtr disk, goto cleanup; if (qemuDomainGetStatsOneBlock(driver, cfg, dom, records, nrecords, - alias, src, *recordnr, + alias, n, *recordnr, stats) < 0) goto cleanup; VIR_FREE(alias); (*recordnr)++; -src = src->backingStore; + +if (!visitBacking) +break; } ret = 0; -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 40/55] qemu: Explicitly find disks for stats totals
Rather than totalling every entry from 'query-block' for stats provided by qemuDomainBlocksStatsGather total only stats for known disks. This will allow to return data for nodenames and qdevs in the same hash so that we can use them with -blockdev. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 25 ++--- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c941e502e5..af347c732c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11037,13 +11037,9 @@ qemuDomainBlockResize(virDomainPtr dom, static int -qemuDomainBlockStatsGatherTotals(void *payload, - const void *name ATTRIBUTE_UNUSED, - void *opaque) +qemuDomainBlockStatsGatherTotals(qemuBlockStatsPtr data, + qemuBlockStatsPtr total) { -qemuBlockStatsPtr data = payload; -qemuBlockStatsPtr total = opaque; - #define QEMU_BLOCK_STAT_TOTAL(NAME) \ if (data->NAME > 0) \ total->NAME += data->NAME @@ -11084,6 +11080,7 @@ qemuDomainBlocksStatsGather(virQEMUDriverPtr driver, virDomainDiskDefPtr disk; virHashTablePtr blockstats = NULL; qemuBlockStatsPtr stats; +size_t i; int nstats; const char *entryname = NULL; int ret = -1; @@ -11125,7 +11122,21 @@ qemuDomainBlocksStatsGather(virQEMUDriverPtr driver, **retstats = *stats; } else { -virHashForEach(blockstats, qemuDomainBlockStatsGatherTotals, *retstats); +for (i = 0; i < vm->def->ndisks; i++) { +disk = vm->def->disks[i]; +entryname = disk->info.alias; + +if (!entryname) +continue; + +if (!(stats = virHashLookup(blockstats, entryname))) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find statistics for device '%s'"), entryname); +goto cleanup; +} + +qemuDomainBlockStatsGatherTotals(stats, *retstats); +} } ret = nstats; -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 27/55] qemu: domain: Add field for storing node name for copy-on-read
The copy-on-read feature is expressed by adding a new node layer in qemu when using -blockdev. Since we will keep these per-disk (as opposed to per storage source) we need to store the appropriate node names in the disk definition. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c| 11 +++ src/qemu/qemu_domain.h| 1 + tests/qemustatusxml2xmldata/modern-in.xml | 3 +++ 3 files changed, 15 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9c1a2c6053..c98be208f1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1066,6 +1066,7 @@ qemuDomainDiskPrivateDispose(void *obj) VIR_FREE(priv->blockJobError); virStorageSourceFree(priv->migrSource); VIR_FREE(priv->backendQomName); +VIR_FREE(priv->nodeCopyOnRead); } static virClassPtr qemuDomainStorageSourcePrivateClass; @@ -2131,6 +2132,7 @@ qemuDomainDiskPrivateParse(xmlXPathContextPtr ctxt, qemuDomainDiskPrivatePtr priv = QEMU_DOMAIN_DISK_PRIVATE(disk); priv->backendQomName = virXPathString("string(./qom/@backend)", ctxt); +priv->nodeCopyOnRead = virXPathString("string(./nodenames/nodename[@type='copyOnRead']/@name)", ctxt); return 0; } @@ -2144,6 +2146,15 @@ qemuDomainDiskPrivateFormat(virDomainDiskDefPtr disk, virBufferEscapeString(buf, "\n", priv->backendQomName); +if (priv->nodeCopyOnRead) { +virBufferAddLit(buf, "\n"); +virBufferAdjustIndent(buf, 2); +virBufferEscapeString(buf, "\n", + priv->nodeCopyOnRead); +virBufferAdjustIndent(buf, -2); +virBufferAddLit(buf, "\n"); +} + return 0; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 7d0b304727..d3147cb69a 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -398,6 +398,7 @@ struct _qemuDomainDiskPrivate { bool removable; /* device media can be removed/changed */ char *backendQomName; /* QOM path to the eligible block backend */ +char *nodeCopyOnRead; /* nodename of the disk-wide copy-on-read blockdev layer */ }; # define QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src) \ diff --git a/tests/qemustatusxml2xmldata/modern-in.xml b/tests/qemustatusxml2xmldata/modern-in.xml index 21d4faca66..612090786a 100644 --- a/tests/qemustatusxml2xmldata/modern-in.xml +++ b/tests/qemustatusxml2xmldata/modern-in.xml @@ -333,6 +333,9 @@ + + + -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 32/55] qemu: command: Add helper to check if disk throttling is enabled
Add a helper which will use a collection of other helpers to determine whether a disk requires throttling to be enabled. Signed-off-by: Peter Krempa --- src/qemu/qemu_command.c | 10 ++ src/qemu/qemu_command.h | 3 +++ 2 files changed, 13 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3cce1bb05e..35cef61720 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1031,6 +1031,16 @@ qemuDiskConfigBlkdeviotuneHasMaxLength(virDomainDiskDefPtr disk) } +bool +qemuDiskConfigBlkdeviotuneEnabled(virDomainDiskDefPtr disk) +{ +return !!disk->blkdeviotune.group_name || + qemuDiskConfigBlkdeviotuneHasBasic(disk) || + qemuDiskConfigBlkdeviotuneHasMax(disk) || + qemuDiskConfigBlkdeviotuneHasMaxLength(disk); +} + + /** * qemuCheckDiskConfigBlkdeviotune: * @disk: disk configuration diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index cf17dc1ede..b8f92a 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -181,6 +181,9 @@ int qemuGetDriveSourceString(virStorageSourcePtr src, qemuDomainSecretInfoPtr secinfo, char **source); +bool +qemuDiskConfigBlkdeviotuneEnabled(virDomainDiskDefPtr disk); + int qemuCheckDiskConfig(virDomainDiskDefPtr disk, virQEMUCapsPtr qemuCaps); -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 36/55] qemu: hotplug: Prepare for blockdev-add/blockdev-del with backing chains
Initialize data for the whole backing chain when plugging in or removing disks when a machine supports -blockdev. Similarly to startup we need to prepare the structures for the whole backing chain and take care of the copy-on-read feature. Signed-off-by: Peter Krempa --- src/qemu/qemu_hotplug.c | 75 - 1 file changed, 61 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 87efd3b411..3eddb0043e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -380,6 +380,10 @@ qemuHotplugRemoveManagedPR(virQEMUDriverPtr driver, struct _qemuHotplugDiskSourceData { qemuBlockStorageSourceAttachDataPtr *backends; size_t nbackends; + +/* disk copy-on-read object */ +virJSONValuePtr corProps; +char *corAlias; }; typedef struct _qemuHotplugDiskSourceData qemuHotplugDiskSourceData; typedef qemuHotplugDiskSourceData *qemuHotplugDiskSourceDataPtr; @@ -393,6 +397,9 @@ qemuHotplugDiskSourceDataFree(qemuHotplugDiskSourceDataPtr data) if (!data) return; +virJSONValueFree(data->corProps); +VIR_FREE(data->corAlias); + for (i = 0; i < data->nbackends; i++) qemuBlockStorageSourceAttachDataFree(data->backends[i]); @@ -461,25 +468,40 @@ qemuHotplugRemoveStorageSourcePrepareData(virStorageSourcePtr src, static qemuHotplugDiskSourceDataPtr qemuHotplugDiskSourceRemovePrepare(virDomainDiskDefPtr disk, - virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED) + virQEMUCapsPtr qemuCaps) { +qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); qemuBlockStorageSourceAttachDataPtr backend = NULL; qemuHotplugDiskSourceDataPtr data = NULL; qemuHotplugDiskSourceDataPtr ret = NULL; char *drivealias = NULL; +virStorageSourcePtr n; if (VIR_ALLOC(data) < 0) return NULL; -if (!(drivealias = qemuAliasDiskDriveFromDisk(disk))) -goto cleanup; +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) { +if (VIR_STRDUP(data->corAlias, diskPriv->nodeCopyOnRead) < 0) +goto cleanup; -if (!(backend = qemuHotplugRemoveStorageSourcePrepareData(disk->src, - drivealias))) -goto cleanup; +for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) { +if (!(backend = qemuHotplugRemoveStorageSourcePrepareData(n, NULL))) +goto cleanup; -if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) < 0) -goto cleanup; +if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) < 0) +goto cleanup; +} +} else { +if (!(drivealias = qemuAliasDiskDriveFromDisk(disk))) +goto cleanup; + +if (!(backend = qemuHotplugRemoveStorageSourcePrepareData(disk->src, + drivealias))) +goto cleanup; + +if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) < 0) +goto cleanup; +} VIR_STEAL_PTR(ret, data); @@ -505,18 +527,36 @@ qemuHotplugDiskSourceAttachPrepare(virDomainDiskDefPtr disk, qemuBlockStorageSourceAttachDataPtr backend; qemuHotplugDiskSourceDataPtr data; qemuHotplugDiskSourceDataPtr ret = NULL; +virStorageSourcePtr n; if (VIR_ALLOC(data) < 0) return NULL; -if (!(backend = qemuBuildStorageSourceAttachPrepareDrive(disk, qemuCaps, false))) -goto cleanup; +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) { +if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON && +!(data->corProps = qemuBlockStorageGetCopyOnReadProps(disk))) +goto cleanup; -if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, backend, qemuCaps) < 0) -goto cleanup; +for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) { +if (!(backend = qemuBlockStorageSourceAttachPrepareBlockdev(n))) +goto cleanup; -if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) < 0) -goto cleanup; +if (qemuBuildStorageSourceAttachPrepareCommon(n, backend, qemuCaps) < 0) +goto cleanup; + +if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) < 0) +goto cleanup; +} +} else { +if (!(backend = qemuBuildStorageSourceAttachPrepareDrive(disk, qemuCaps, false))) +goto cleanup; + +if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, backend, qemuCaps) < 0) +goto cleanup; + +if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) < 0) +goto cleanup; +} VIR_STEAL_PTR(ret, data); @@ -546,6 +586,10 @@ qemuHotplugDiskSourceAttach(qemuMonitorPtr mon, return -1;
[libvirt] [PATCH 39/55] qemu: driver: Don't copy disk alias in qemuDomainBlocksStatsGather
The string is not modified so it does not make sense to have a copy. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7541beb718..c941e502e5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11085,7 +11085,7 @@ qemuDomainBlocksStatsGather(virQEMUDriverPtr driver, virHashTablePtr blockstats = NULL; qemuBlockStatsPtr stats; int nstats; -char *diskAlias = NULL; +const char *entryname = NULL; int ret = -1; if (*path) { @@ -11100,8 +11100,7 @@ qemuDomainBlocksStatsGather(virQEMUDriverPtr driver, goto cleanup; } -if (VIR_STRDUP(diskAlias, disk->info.alias) < 0) -goto cleanup; +entryname = disk->info.alias; } qemuDomainObjEnterMonitor(driver, vm); @@ -7,10 +6,10 @@ qemuDomainBlocksStatsGather(virQEMUDriverPtr driver, if (VIR_ALLOC(*retstats) < 0) goto cleanup; -if (diskAlias) { -if (!(stats = virHashLookup(blockstats, diskAlias))) { +if (entryname) { +if (!(stats = virHashLookup(blockstats, entryname))) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find statistics for device '%s'"), diskAlias); + _("cannot find statistics for device '%s'"), entryname); goto cleanup; } @@ -11132,7 +11131,6 @@ qemuDomainBlocksStatsGather(virQEMUDriverPtr driver, ret = nstats; cleanup: -VIR_FREE(diskAlias); virHashFree(blockstats); return ret; } -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 30/55] qemu: domain: Prepare qemuDomainDiskGetBackendAlias for -blockdev
Pass in the node name as the backend alias when -blockdev is used. As copy-on-read is expressed by a separate -blockdev backing chain member we need to decide which node name to use here. For empty cdroms when using -blockdev there is no backend at all so NULL is returned. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 21 +++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 526634d819..ea3929ce7e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8676,12 +8676,29 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, */ int qemuDomainDiskGetBackendAlias(virDomainDiskDefPtr disk, - virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED, + virQEMUCapsPtr qemuCaps, char **backendAlias) { +qemuDomainDiskPrivatePtr priv = QEMU_DOMAIN_DISK_PRIVATE(disk); +const char *nodename = NULL; *backendAlias = NULL; -if (!(*backendAlias = qemuAliasDiskDriveFromDisk(disk))) +if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) { +if (!(*backendAlias = qemuAliasDiskDriveFromDisk(disk))) +return -1; + +return 0; +} + +if (virStorageSourceIsEmpty(disk->src)) +return 0; + +if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON) +nodename = priv->nodeCopyOnRead; +else +nodename = disk->src->nodeformat; + +if (VIR_STRDUP(*backendAlias, nodename) < 0) return -1; return 0; -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 33/55] qemu: process: Setup disk io throttling for -blockdev
The proper way to do this would be to use the 'throttle' driver but unfortunately it can't change the 'throttle_group' so we can't provide feature parity. This hack uses the block_set_io_throttle command to do so until we can properly replace it. Signed-off-by: Peter Krempa --- src/qemu/qemu_process.c | 50 + 1 file changed, 50 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 334044f479..2183127133 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6233,6 +6233,53 @@ qemuProcessGenID(virDomainObjPtr vm, } +/** + * qemuProcessSetupDiskThrottlingBlockdev: + * + * Sets up disk trottling for -blockdev via block_set_io_throttle monitor + * command. This hack should be replaced by proper use of the 'throttle' + * blockdev driver in qemu once it will support changing of the throttle group. + */ +static int +qemuProcessSetupDiskThrottlingBlockdev(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob) +{ +qemuDomainObjPrivatePtr priv = vm->privateData; +size_t i; +int ret = -1; + +if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) +return 0; + +VIR_DEBUG("Setting up disk throttling for -blockdev via block_set_io_throttle"); + +if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) +return -1; + +for (i = 0; i < vm->def->ndisks; i++) { +virDomainDiskDefPtr disk = vm->def->disks[i]; +qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + +if (!qemuDiskConfigBlkdeviotuneEnabled(disk)) +continue; + +if (qemuMonitorSetBlockIoThrottle(qemuDomainGetMonitor(vm), NULL, + diskPriv->backendQomName, + &disk->blkdeviotune, + true, true, true) < 0) +goto cleanup; +} + +ret = 0; + + cleanup: +if (qemuDomainObjExitMonitor(driver, vm) < 0) +ret = -1; +return ret; +} + + /** * qemuProcessLaunch: * @@ -6551,6 +6598,9 @@ qemuProcessLaunch(virConnectPtr conn, if (qemuProcessSetupBalloon(driver, vm, asyncJob) < 0) goto cleanup; +if (qemuProcessSetupDiskThrottlingBlockdev(driver, vm, asyncJob) < 0) +goto cleanup; + /* Since CPUs were not started yet, the balloon could not return the memory * to the host and thus cur_balloon needs to be updated so that GetXMLdesc * and friends return the correct size in case they can't grab the job */ -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 34/55] qemu: driver: Use QOM backend name for disk IO throttling APIs
With -blockdev the drive alias can't be used any more so we need to switch to the QOM name. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 30 -- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8e941d7a60..f994eb1538 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18216,7 +18216,8 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, virDomainDefPtr def = NULL; virDomainDefPtr persistentDef = NULL; virDomainBlockIoTuneInfo info; -char *device = NULL; +char *drivealias = NULL; +const char *qdevid = NULL; int ret = -1; size_t i; virDomainDiskDefPtr conf_disk = NULL; @@ -18441,8 +18442,12 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, if (!(disk = qemuDomainDiskByName(def, path))) goto endjob; -if (!(device = qemuAliasDiskDriveFromDisk(disk))) -goto endjob; +if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) { +qdevid = QEMU_DOMAIN_DISK_PRIVATE(disk)->backendQomName; +} else { +if (!(drivealias = qemuAliasDiskDriveFromDisk(disk))) +goto endjob; +} if (qemuDomainSetBlockIoTuneDefaults(&info, &disk->blkdeviotune, set_fields) < 0) @@ -18488,7 +18493,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, * via the JSON error code from the block_set_io_throttle call */ qemuDomainObjEnterMonitor(driver, vm); -ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, NULL, +ret = qemuMonitorSetBlockIoThrottle(priv->mon, drivealias, qdevid, &info, supportMaxOptions, set_fields & QEMU_BLOCK_IOTUNE_SET_GROUP_NAME, supportMaxLengthOptions); @@ -18538,7 +18543,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, cleanup: VIR_FREE(info.group_name); -VIR_FREE(device); +VIR_FREE(drivealias); virDomainObjEndAPI(&vm); if (eventNparams) virTypedParamsFree(eventParams, eventNparams); @@ -18560,7 +18565,8 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, virDomainDefPtr def = NULL; virDomainDefPtr persistentDef = NULL; virDomainBlockIoTuneInfo reply = {0}; -char *device = NULL; +char *drivealias = NULL; +const char *qdevid = NULL; int ret = -1; int maxparams; @@ -18614,10 +18620,14 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, if (!(disk = qemuDomainDiskByName(def, path))) goto endjob; -if (!(device = qemuAliasDiskDriveFromDisk(disk))) -goto endjob; +if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) { +qdevid = QEMU_DOMAIN_DISK_PRIVATE(disk)->backendQomName; +} else { +if (!(drivealias = qemuAliasDiskDriveFromDisk(disk))) +goto endjob; +} qemuDomainObjEnterMonitor(driver, vm); -ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, NULL, &reply); +ret = qemuMonitorGetBlockIoThrottle(priv->mon, drivealias, qdevid, &reply); if (qemuDomainObjExitMonitor(driver, vm) < 0) goto endjob; if (ret < 0) @@ -18692,7 +18702,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, cleanup: VIR_FREE(reply.group_name); -VIR_FREE(device); +VIR_FREE(drivealias); virDomainObjEndAPI(&vm); return ret; } -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 37/55] qemu: monitor: Add APIs for cdrom tray handling for -blockdev
With blockdev we can use the full range of commands to manipulate the tray and the medium separately. Implement monitor code for this. Schema testing done in the qemumonitorjsontest allows us to verify that we generate the commands correctly. Signed-off-by: Peter Krempa --- src/qemu/qemu_monitor.c | 51 +++ src/qemu/qemu_monitor.h | 14 ++ src/qemu/qemu_monitor_json.c | 114 +++ src/qemu/qemu_monitor_json.h | 18 +++ tests/qemumonitorjsontest.c | 8 +++ 5 files changed, 205 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 4dcbd69dce..d665f33d81 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4334,6 +4334,57 @@ qemuMonitorBlockdevDel(qemuMonitorPtr mon, return qemuMonitorJSONBlockdevDel(mon, nodename); } +int +qemuMonitorBlockdevTrayOpen(qemuMonitorPtr mon, +const char *id, +bool force) +{ +VIR_DEBUG("id=%s force=%d", id, force); + +QEMU_CHECK_MONITOR(mon); + +return qemuMonitorJSONBlockdevTrayOpen(mon, id, force); +} + + +int +qemuMonitorBlockdevTrayClose(qemuMonitorPtr mon, + const char *id) +{ +VIR_DEBUG("id=%s", id); + +QEMU_CHECK_MONITOR(mon); + +return qemuMonitorJSONBlockdevTrayClose(mon, id); +} + + +int +qemuMonitorBlockdevMediumRemove(qemuMonitorPtr mon, +const char *id) +{ +VIR_DEBUG("id=%s", id); + +QEMU_CHECK_MONITOR(mon); + +return qemuMonitorJSONBlockdevMediumRemove(mon, id); +} + + + +int +qemuMonitorBlockdevMediumInsert(qemuMonitorPtr mon, +const char *id, +const char *nodename) +{ +VIR_DEBUG("id=%s nodename=%s", id, nodename); + +QEMU_CHECK_MONITOR(mon); + +return qemuMonitorJSONBlockdevMediumInsert(mon, id, nodename); +} + + char * qemuMonitorGetSEVMeasurement(qemuMonitorPtr mon) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index f8f6969ddb..0734588f66 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1151,6 +1151,20 @@ int qemuMonitorBlockdevAdd(qemuMonitorPtr mon, int qemuMonitorBlockdevDel(qemuMonitorPtr mon, const char *nodename); +int qemuMonitorBlockdevTrayOpen(qemuMonitorPtr mon, +const char *id, +bool force); + +int qemuMonitorBlockdevTrayClose(qemuMonitorPtr mon, + const char *id); + +int qemuMonitorBlockdevMediumRemove(qemuMonitorPtr mon, +const char *id); + +int qemuMonitorBlockdevMediumInsert(qemuMonitorPtr mon, +const char *id, +const char *nodename); + char * qemuMonitorGetSEVMeasurement(qemuMonitorPtr mon); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index de3fcd83d8..105f6f78ac 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8004,6 +8004,120 @@ qemuMonitorJSONBlockdevDel(qemuMonitorPtr mon, return ret; } + +int +qemuMonitorJSONBlockdevTrayOpen(qemuMonitorPtr mon, +const char *id, +bool force) +{ +virJSONValuePtr cmd; +virJSONValuePtr reply = NULL; +int ret = -1; + +if (!(cmd = qemuMonitorJSONMakeCommand("blockdev-open-tray", + "s:id", id, + "b:force", force, NULL))) +return -1; + +if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) +goto cleanup; + +if (qemuMonitorJSONCheckError(cmd, reply) < 0) +goto cleanup; + +ret = 0; + + cleanup: +virJSONValueFree(cmd); +virJSONValueFree(reply); +return ret; +} + + +int +qemuMonitorJSONBlockdevTrayClose(qemuMonitorPtr mon, + const char *id) +{ +virJSONValuePtr cmd; +virJSONValuePtr reply = NULL; +int ret = -1; + +if (!(cmd = qemuMonitorJSONMakeCommand("blockdev-close-tray", + "s:id", id, NULL))) +return -1; + +if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) +goto cleanup; + +if (qemuMonitorJSONCheckError(cmd, reply) < 0) +goto cleanup; + +ret = 0; + + cleanup: +virJSONValueFree(cmd); +virJSONValueFree(reply); +return ret; +} + + +int +qemuMonitorJSONBlockdevMediumRemove(qemuMonitorPtr mon, +const char *id) +{ +virJSONValuePtr cmd; +virJSONValuePtr reply = NULL; +int ret = -1; + +if (!(cmd = qemuMonitorJSONMakeCommand("blockdev-remove-medium", + "s:id", id, NULL))) +return -1; + +if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) +goto cleanup; + +if (qemuMonitorJS
[libvirt] [PATCH 05/55] tests: qemucapabilities: Update capability data for qemu 3.0.0
The diff contains changes from the change of the JSON library reformatting as well as dropping of the preconfig state and adding of the 'qdev' field to output of 'query-blockstats'. Signed-off-by: Peter Krempa --- .../qemucapabilitiesdata/caps_3.0.0.x86_64.replies | 591 +++-- tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml | 8 +- 2 files changed, 88 insertions(+), 511 deletions(-) diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.replies index b2f8377248..714fac5a6f 100644 --- a/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.replies @@ -4,8 +4,7 @@ } { - "return": { - }, + "return": {}, "id": "libvirt-1" } @@ -17,11 +16,11 @@ { "return": { "qemu": { - "micro": 90, + "micro": 93, "minor": 12, "major": 2 }, -"package": "v3.0.0-rc0-31-g633e824037" +"package": "v3.0.0-rc3-17-g09b94ac0f2" }, "id": "libvirt-2" } @@ -175,7 +174,7 @@ "name": "system_wakeup" }, { - "name": "exit-preconfig" + "name": "x-exit-preconfig" }, { "name": "cont" @@ -5346,40 +5345,35 @@ { "name": "max", "typename": "max-x86_64-cpu", - "unavailable-features": [ - ], + "unavailable-features": [], "static": false, "migration-safe": false }, { "name": "host", "typename": "host-x86_64-cpu", - "unavailable-features": [ - ], + "unavailable-features": [], "static": false, "migration-safe": false }, { "name": "base", "typename": "base-x86_64-cpu", - "unavailable-features": [ - ], + "unavailable-features": [], "static": true, "migration-safe": true }, { "name": "qemu64", "typename": "qemu64-x86_64-cpu", - "unavailable-features": [ - ], + "unavailable-features": [], "static": false, "migration-safe": true }, { "name": "qemu32", "typename": "qemu32-x86_64-cpu", - "unavailable-features": [ - ], + "unavailable-features": [], "static": false, "migration-safe": true }, @@ -5400,64 +5394,56 @@ { "name": "pentium3", "typename": "pentium3-x86_64-cpu", - "unavailable-features": [ - ], + "unavailable-features": [], "static": false, "migration-safe": true }, { "name": "pentium2", "typename": "pentium2-x86_64-cpu", - "unavailable-features": [ - ], + "unavailable-features": [], "static": false, "migration-safe": true }, { "name": "pentium", "typename": "pentium-x86_64-cpu", - "unavailable-features": [ - ], + "unavailable-features": [], "static": false, "migration-safe": true }, { "name": "n270", "typename": "n270-x86_64-cpu", - "unavailable-features": [ - ], + "unavailable-features": [], "static": false, "migration-safe": true }, { "name": "kvm64", "typename": "kvm64-x86_64-cpu", - "unavailable-features": [ - ], + "unavailable-features": [], "static": false, "migration-safe": true }, { "name": "kvm32", "typename": "kvm32-x86_64-cpu", - "unavailable-features": [ - ], + "unavailable-features": [], "static": false, "migration-safe": true }, { "name": "coreduo", "typename": "coreduo-x86_64-cpu", - "unavailable-features": [ - ], + "unavailable-features": [], "static": false, "migration-safe": true }, { "name": "core2duo", "typename": "core2duo-x86_64-cpu", - "unavailable-features": [ - ], + "unavailable-features": [], "static": false, "migration-safe": true }, @@ -5475,16 +5461,14 @@ { "name": "Westmere-IBRS", "typename": "Westmere-IBRS-x86_64-cpu", - "unavailable-features": [ - ], + "unavailable-features": [], "static": false, "migration-safe": true }, { "name": "Westmere", "typename": "Westmere-x86_64-cpu", - "unavailable-features": [ - ], + "unavailable-features": [], "static": false, "migration-safe": true }, @@ -5525,40 +5509,35 @@ { "name": "Skylake-Client-IBRS", "typename": "Skylake-Client-IBRS-x86_64-cpu", - "unavailable-features": [ - ], + "unavailable-features": [], "static": false, "migration-safe": true }, { "name": "Skylake-Client", "typename": "Skylake-Client-x86_64-cpu", - "unavailable-features": [ - ], + "unavailable-features": [], "static": false, "migration-safe": true }, { "name": "SandyBridge-IBRS",
[libvirt] [PATCH 12/55] tests: qemuxml2argv: Fork CAPS_LATEST test cases for 'blockdev'
The blockdev support will change existing approach to add disks to VMs so all tests using the DO_TEST_CAPS_LATEST approach which have any disks need to be forked so that the changes can be applied. Signed-off-by: Peter Krempa --- tests/qemuxml2argvdata/disk-aio.x86_64-2.12.0.args | 37 + .../qemuxml2argvdata/disk-cache.x86_64-2.12.0.args | 50 + .../disk-cdrom-network.x86_64-2.12.0.args | 41 ++ .../disk-cdrom-tray.x86_64-2.12.0.args | 39 ++ .../qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args | 35 .../disk-copy_on_read.x86_64-2.12.0.args | 41 ++ .../disk-detect-zeroes.x86_64-2.12.0.args | 37 + .../disk-error-policy.x86_64-2.12.0.args | 41 ++ .../disk-floppy.x86_64-2.12.0.args | 35 .../disk-network-gluster.x86_64-2.12.0.args| 44 +++ .../disk-network-iscsi.x86_64-2.12.0.args | 63 ++ .../disk-network-nbd.x86_64-2.12.0.args| 46 .../disk-network-rbd.x86_64-2.12.0.args| 61 + .../disk-network-sheepdog.x86_64-2.12.0.args | 35 .../disk-network-source-auth.x86_64-2.12.0.args| 47 .../disk-network-tlsx509.x86_64-2.12.0.args| 59 .../disk-readonly-disk.x86_64-2.12.0.args | 34 .../disk-shared.x86_64-2.12.0.args | 37 + ...isk-virtio-scsi-reservations.x86_64-2.12.0.args | 43 +++ .../floppy-drive-fat.x86_64-2.12.0.args| 33 tests/qemuxml2argvtest.c | 20 +++ 21 files changed, 878 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-aio.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-cache.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-cdrom-network.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-cdrom-tray.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-copy_on_read.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-detect-zeroes.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-error-policy.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-floppy.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-network-gluster.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-network-iscsi.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-network-nbd.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-network-rbd.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-network-sheepdog.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-network-source-auth.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-network-tlsx509.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-readonly-disk.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-shared.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-reservations.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/floppy-drive-fat.x86_64-2.12.0.args diff --git a/tests/qemuxml2argvdata/disk-aio.x86_64-2.12.0.args b/tests/qemuxml2argvdata/disk-aio.x86_64-2.12.0.args new file mode 100644 index 00..1dfade0882 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-aio.x86_64-2.12.0.args @@ -0,0 +1,37 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc-i440fx-2.12,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-realtime mlock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-ide0-0-0,\ +cache=none,aio=native \ +-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1,\ +write-cache=on \ +-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-1-0,\ +readonly=on,aio=threads \ +-device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/disk-cache.x86_64-2.12.0.args b/tests/qemuxml2ar
[libvirt] [PATCH 31/55] qemu: command: format disk source commandline for -blockdev
Format the backing chain onto the commandline using the 'json' syntax with -blockdev. The command line formatter needs only minor tweaks to add the new entries but we now need to initialize the strucutres that are used for every layer of the backing chain. Signed-off-by: Peter Krempa --- src/qemu/qemu_command.c | 84 - 1 file changed, 76 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d148db90fa..3cce1bb05e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2237,6 +2237,8 @@ static int qemuBuildBlockStorageSourceAttachDataCommandline(virCommandPtr cmd, qemuBlockStorageSourceAttachDataPtr data) { +char *tmp; + if (qemuBuildObjectCommandline(cmd, data->prmgrProps) < 0 || qemuBuildObjectCommandline(cmd, data->authsecretProps) < 0 || qemuBuildObjectCommandline(cmd, data->encryptsecretProps) < 0 || @@ -2246,6 +2248,22 @@ qemuBuildBlockStorageSourceAttachDataCommandline(virCommandPtr cmd, if (data->driveCmd) virCommandAddArgList(cmd, "-drive", data->driveCmd, NULL); +if (data->storageProps) { +if (!(tmp = virJSONValueToString(data->storageProps, false))) +return -1; + +virCommandAddArgList(cmd, "-blockdev", tmp, NULL); +VIR_FREE(tmp); +} + +if (data->formatProps) { +if (!(tmp = virJSONValueToString(data->formatProps, false))) +return -1; + +virCommandAddArgList(cmd, "-blockdev", tmp, NULL); +VIR_FREE(tmp); +} + return 0; } @@ -2256,21 +2274,71 @@ qemuBuildDiskSourceCommandLine(virCommandPtr cmd, virQEMUCapsPtr qemuCaps, bool driveBoot) { -qemuBlockStorageSourceAttachDataPtr data = NULL; +qemuBlockStorageSourceAttachDataPtr *data = NULL; +size_t ndata = 0; +qemuBlockStorageSourceAttachDataPtr tmp = NULL; +virJSONValuePtr copyOnReadProps = NULL; +virStorageSourcePtr n; +char *str = NULL; +size_t i; int ret = -1; -if (!(data = qemuBuildStorageSourceAttachPrepareDrive(disk, qemuCaps, - driveBoot))) -return -1; +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) { +if (virStorageSourceIsEmpty(disk->src)) { +ret = 0; +goto cleanup; +} -if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, data, qemuCaps) < 0 || -qemuBuildBlockStorageSourceAttachDataCommandline(cmd, data) < 0) -goto cleanup; +for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) { +if (!(tmp = qemuBlockStorageSourceAttachPrepareBlockdev(n))) +goto cleanup; + +if (qemuBuildStorageSourceAttachPrepareCommon(n, tmp, qemuCaps) < 0) +goto cleanup; + +if (VIR_APPEND_ELEMENT(data, ndata, tmp) < 0) +goto cleanup; +} + +if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON && +!(copyOnReadProps = qemuBlockStorageGetCopyOnReadProps(disk))) +goto cleanup; +} else { +if (!(tmp = qemuBuildStorageSourceAttachPrepareDrive(disk, qemuCaps, + driveBoot))) +goto cleanup; + +if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, tmp, + qemuCaps) < 0) +goto cleanup; + +if (VIR_APPEND_ELEMENT(data, ndata, tmp) < 0) +goto cleanup; +} + +for (i = ndata; i > 0; i--) { +if (qemuBuildBlockStorageSourceAttachDataCommandline(cmd, + data[i - 1]) < 0) +goto cleanup; +} + +if (copyOnReadProps) { +if (!(str = virJSONValueToString(copyOnReadProps, false))) +goto cleanup; + +virCommandAddArgList(cmd, "-blockdev", str, NULL); +VIR_FREE(str); +} ret = 0; cleanup: -qemuBlockStorageSourceAttachDataFree(data); +for (i = 0; i < ndata; i++) +qemuBlockStorageSourceAttachDataFree(data[i]); +VIR_FREE(data); +qemuBlockStorageSourceAttachDataFree(tmp); +virJSONValueFree(copyOnReadProps); +VIR_FREE(str); return ret; } -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 23/55] conf: Allow formatting and parsing of 'index' for disk source image
Similarly to backing store indexes which will become stable eventually we need also to be able to format and store in the status XML for later use the index for the top level of the backing chain. Add XML formatter, parser, schema and docs. Signed-off-by: Peter Krempa --- docs/formatdomain.html.in | 7 ++- docs/schemas/domaincommon.rng | 19 +++ src/conf/domain_conf.c | 21 + .../qemuxml2argvdata/disk-backing-chains-index.xml | 12 ++-- .../disk-backing-chains-index-active.xml| 12 ++-- 5 files changed, 54 insertions(+), 17 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index b63467bd91..f175d931ec 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2951,6 +2951,11 @@ is only valid when the specified storage volume is of 'file' or 'block' type). +The source element may also have the index +attribute with same semantics the +index attribute of backingStore + + The source element may contain the following sub elements: @@ -3150,7 +3155,7 @@ by the backing store, see disk type attribute above for more details and possible values. - index + index This attribute is only valid in output (and ignored on input) and it can be used to refer to a specific part of the disk chain when diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index ac04af51a1..7c4e848685 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1528,6 +1528,14 @@ + + + + + + + + @@ -1551,6 +1559,7 @@ + @@ -1575,6 +1584,7 @@ + @@ -1600,6 +1610,7 @@ + @@ -1653,6 +1664,7 @@ rbd + @@ -1692,6 +1704,7 @@ iscsi + @@ -1711,6 +1724,7 @@ + @@ -1729,6 +1743,7 @@ + @@ -1749,6 +1764,7 @@ + @@ -1762,6 +1778,7 @@ gluster + @@ -1779,6 +1796,7 @@ + @@ -1823,6 +1841,7 @@ + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a9db7cd380..720b418920 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9677,6 +9677,13 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, startupPolicy = virXMLPropString(cur, "startupPolicy"); +if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) && +(tmp = virXMLPropString(cur, "index")) && +virStrToLong_uip(tmp, NULL, 10, &def->src->id) < 0) { +virReportError(VIR_ERR_XML_ERROR, _("invalid disk index '%s'"), tmp); +goto error; +} +VIR_FREE(tmp); } else if (!target && virXMLNodeNameEqual(cur, "target")) { target = virXMLPropString(cur, "dev"); @@ -23681,6 +23688,7 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf, int policy, unsigned int flags, bool skipSeclabels, + bool attrIndex, virDomainXMLOptionPtr xmlopt) { virBuffer attrBuf = VIR_BUFFER_INITIALIZER; @@ -23697,6 +23705,9 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf, virBufferEscapeString(&attrBuf, " startupPolicy='%s'", virDomainStartupPolicyTypeToString(policy)); +if (attrIndex && src->id != 0) +virBufferAsprintf(&attrBuf, " index='%u'", src->id); + if (virDomainDiskSourceFormatPrivateData(&childBuf, src, flags, xmlopt) < 0) goto cleanup; @@ -23719,7 +23730,8 @@ virDomainDiskSourceFormat(virBufferPtr buf, unsigned int flags, virDomainXMLOptionPtr xmlopt) { -return virDomainDiskSourceFormatInternal(buf, src, policy, flags, false, xmlopt); +return virDomainDiskSourceFormatInternal(buf, src, policy, flags, false, + false, xmlopt); } @@ -23761,7 +23773,8 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf, virBufferAsprintf(
[libvirt] [PATCH 07/55] qemu: monitor: Reuse qemuMonitorJSONQueryBlock in qemuMonitorJSONBlockIoThrottleInfo
The wrapper executes the command and does error detection so there's no need to open-code all of those things. Signed-off-by: Peter Krempa --- src/qemu/qemu_monitor_json.c | 37 + 1 file changed, 5 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index e20851eeb2..48439ccae1 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4817,21 +4817,14 @@ int qemuMonitorJSONOpenGraphics(qemuMonitorPtr mon, goto cleanup; \ } static int -qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, +qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr io_throttle, const char *device, virDomainBlockIoTuneInfoPtr reply) { -virJSONValuePtr io_throttle; int ret = -1; size_t i; bool found = false; -if (!(io_throttle = virJSONValueObjectGetArray(result, "return"))) { -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _(" block_io_throttle reply was missing device list")); -goto cleanup; -} - for (i = 0; i < virJSONValueArraySize(io_throttle); i++) { virJSONValuePtr temp_dev = virJSONValueArrayGet(io_throttle, i); virJSONValuePtr inserted; @@ -5001,33 +4994,13 @@ int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon, virDomainBlockIoTuneInfoPtr reply) { int ret = -1; -virJSONValuePtr cmd = NULL; -virJSONValuePtr result = NULL; +virJSONValuePtr devices = NULL; -cmd = qemuMonitorJSONMakeCommand("query-block", NULL); -if (!cmd) +if (!(devices = qemuMonitorJSONQueryBlock(mon))) return -1; -if (qemuMonitorJSONCommand(mon, cmd, &result) < 0) -goto cleanup; - -if (virJSONValueObjectHasKey(result, "error")) { -if (qemuMonitorJSONHasError(result, "DeviceNotActive")) -virReportError(VIR_ERR_OPERATION_INVALID, - _("No active operation on device: %s"), device); -else if (qemuMonitorJSONHasError(result, "NotSupported")) -virReportError(VIR_ERR_OPERATION_INVALID, - _("Operation is not supported for device: %s"), device); -else -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unexpected error")); -goto cleanup; -} - -ret = qemuMonitorJSONBlockIoThrottleInfo(result, device, reply); - cleanup: -virJSONValueFree(cmd); -virJSONValueFree(result); +ret = qemuMonitorJSONBlockIoThrottleInfo(devices, device, reply); +virJSONValueFree(devices); return ret; } -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 20/55] conf: domain: Format out user provided backing chains in XML
If a user configures the backing chain in the XML we should not ignore it. We already do parse it but don't format it out. As a safety-precaution don't attempt to format detected chain into the inactive XML. Signed-off-by: Peter Krempa --- src/conf/domain_conf.c | 8 ++- .../disk-backing-chains-inactive.xml | 35 ++ .../disk-backing-chains-index-active.xml | 80 ++ .../disk-backing-chains-index-inactive.xml | 80 ++ .../disk-backing-chains-noindex-active.xml | 80 ++ .../disk-backing-chains-noindex-inactive.xml | 80 ++ tests/qemuxml2xmloutdata/disk-mirror-inactive.xml | 4 ++ .../disk-mirror-old-inactive.xml | 4 ++ 8 files changed, 369 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7dcbe8a20b..82e47464f2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -23702,10 +23702,15 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf, unsigned int flags) { const char *format; +bool inactive = flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE; if (!backingStore) return 0; +/* don't write detected backing chain members to inactive xml */ +if (inactive && backingStore->detected) +return 0; + if (backingStore->type == VIR_STORAGE_TYPE_NONE) { virBufferAddLit(buf, "\n"); return 0; @@ -23971,8 +23976,7 @@ virDomainDiskDefFormat(virBufferPtr buf, /* Don't format backingStore to inactive XMLs until the code for * persistent storage of backing chains is ready. */ -if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE) && -virDomainDiskBackingStoreFormat(buf, def->src->backingStore, +if (virDomainDiskBackingStoreFormat(buf, def->src->backingStore, xmlopt, flags) < 0) return -1; diff --git a/tests/qemuxml2xmloutdata/disk-backing-chains-inactive.xml b/tests/qemuxml2xmloutdata/disk-backing-chains-inactive.xml index a9db12ba4d..c1af58ff6f 100644 --- a/tests/qemuxml2xmloutdata/disk-backing-chains-inactive.xml +++ b/tests/qemuxml2xmloutdata/disk-backing-chains-inactive.xml @@ -19,6 +19,10 @@ + + + + @@ -27,6 +31,31 @@ + + + + + + + + + + + + + + + + + + + + + + + + + @@ -35,6 +64,7 @@ + @@ -48,6 +78,11 @@ + + + + + diff --git a/tests/qemuxml2xmloutdata/disk-backing-chains-index-active.xml b/tests/qemuxml2xmloutdata/disk-backing-chains-index-active.xml index db70ae2b53..724afa4e83 100644 --- a/tests/qemuxml2xmloutdata/disk-backing-chains-index-active.xml +++ b/tests/qemuxml2xmloutdata/disk-backing-chains-index-active.xml @@ -19,6 +19,10 @@ + + + + @@ -27,6 +31,31 @@ + + + + + + + + + + + + + + + + + + + + + + + + + @@ -35,6 +64,7 @@ + @@ -48,6 +78,11 @@ + + + + + @@ -60,6 +95,51 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/qemuxml2xmlou
[libvirt] [PATCH 25/55] qemu: Add field to store QDEV path of a disk in private data
When using -blockdev you need to use the qdev path to refer to the disk fronends. Add means for storing the path and getting it after restart. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c| 27 +++ src/qemu/qemu_domain.h| 2 ++ tests/qemustatusxml2xmldata/modern-in.xml | 3 +++ 3 files changed, 32 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6103b86478..9c1a2c6053 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1065,6 +1065,7 @@ qemuDomainDiskPrivateDispose(void *obj) VIR_FREE(priv->blockJobError); virStorageSourceFree(priv->migrSource); +VIR_FREE(priv->backendQomName); } static virClassPtr qemuDomainStorageSourcePrivateClass; @@ -2123,6 +2124,30 @@ qemuStorageSourcePrivateDataFormat(virStorageSourcePtr src, } +static int +qemuDomainDiskPrivateParse(xmlXPathContextPtr ctxt, + virDomainDiskDefPtr disk) +{ +qemuDomainDiskPrivatePtr priv = QEMU_DOMAIN_DISK_PRIVATE(disk); + +priv->backendQomName = virXPathString("string(./qom/@backend)", ctxt); + +return 0; +} + + +static int +qemuDomainDiskPrivateFormat(virDomainDiskDefPtr disk, +virBufferPtr buf) +{ +qemuDomainDiskPrivatePtr priv = QEMU_DOMAIN_DISK_PRIVATE(disk); + +virBufferEscapeString(buf, "\n", priv->backendQomName); + +return 0; +} + + static void qemuDomainObjPrivateXMLFormatVcpus(virBufferPtr buf, virDomainDefPtr def) @@ -2973,6 +2998,8 @@ virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks = { .alloc = qemuDomainObjPrivateAlloc, .free = qemuDomainObjPrivateFree, .diskNew = qemuDomainDiskPrivateNew, +.diskParse = qemuDomainDiskPrivateParse, +.diskFormat = qemuDomainDiskPrivateFormat, .vcpuNew = qemuDomainVcpuPrivateNew, .chrSourceNew = qemuDomainChrSourcePrivateNew, .vsockNew = qemuDomainVsockPrivateNew, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 5dcbdfbbe1..7d0b304727 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -396,6 +396,8 @@ struct _qemuDomainDiskPrivate { /* information about the device */ bool tray; /* device has tray */ bool removable; /* device media can be removed/changed */ + +char *backendQomName; /* QOM path to the eligible block backend */ }; # define QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src) \ diff --git a/tests/qemustatusxml2xmldata/modern-in.xml b/tests/qemustatusxml2xmldata/modern-in.xml index 4fb5f326c2..21d4faca66 100644 --- a/tests/qemustatusxml2xmldata/modern-in.xml +++ b/tests/qemustatusxml2xmldata/modern-in.xml @@ -331,6 +331,9 @@ + + + -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 22/55] conf: Implement private data formatting and parsing for disks
Allow storing of private data in the status XML for disks. Signed-off-by: Peter Krempa --- src/conf/domain_conf.c | 60 ++ src/conf/domain_conf.h | 7 ++ 2 files changed, 67 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 82e47464f2..a9db7cd380 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9548,6 +9548,30 @@ virDomainDiskDefDriverParseXML(virDomainDiskDefPtr def, } +static int +virDomainDiskDefParsePrivateData(xmlXPathContextPtr ctxt, + virDomainDiskDefPtr disk, + virDomainXMLOptionPtr xmlopt) +{ +xmlNodePtr save_node = ctxt->node; +int ret = 0; + +if (!xmlopt || +!xmlopt->privateData.diskParse) +return 0; + +if (!(ctxt->node = virXPathNode("./privateData", ctxt))) +goto cleanup; + +if (xmlopt->privateData.diskParse(ctxt, disk) < 0) +ret = -1; + + cleanup: +ctxt->node = save_node; +return ret; +} + + #define VENDOR_LEN 8 #define PRODUCT_LEN 16 @@ -9963,6 +9987,10 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } +if (flags & VIR_DOMAIN_DEF_PARSE_STATUS && +virDomainDiskDefParsePrivateData(ctxt, def, xmlopt) < 0) +goto error; + if (virDomainDiskDefParseValidate(def, vmSeclabels, nvmSeclabels) < 0) goto error; @@ -23911,6 +23939,35 @@ virDomainDiskDefFormatMirror(virBufferPtr buf, } +static int +virDomainDiskDefFormatPrivateData(virBufferPtr buf, + virDomainDiskDefPtr disk, + unsigned int flags, + virDomainXMLOptionPtr xmlopt) +{ +virBuffer childBuf = VIR_BUFFER_INITIALIZER; + +if (!(flags & VIR_DOMAIN_DEF_FORMAT_STATUS) || +!xmlopt || +!xmlopt->privateData.diskFormat) +return 0; + +virBufferSetChildIndent(&childBuf, buf); + +if (xmlopt->privateData.diskFormat(disk, &childBuf) < 0) +goto error; + +if (virXMLFormatElement(buf, "privateData", NULL, &childBuf) < 0) +goto error; + +return 0; + + error: +virBufferFreeAndReset(&childBuf); +return -1; +} + + static int virDomainDiskDefFormat(virBufferPtr buf, virDomainDiskDefPtr def, @@ -24024,6 +24081,9 @@ virDomainDiskDefFormat(virBufferPtr buf, virDomainDeviceInfoFormat(buf, &def->info, flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT); +if (virDomainDiskDefFormatPrivateData(buf, def, flags, xmlopt) < 0) +return -1; + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "\n"); return 0; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c1dfa37fdf..50d2ead607 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2735,6 +2735,11 @@ typedef int (*virDomainXMLPrivateDataParseFunc)(xmlXPathContextPtr, typedef void *(*virDomainXMLPrivateDataGetParseOpaqueFunc)(virDomainObjPtr vm); +typedef int (*virDomainXMLPrivateDataDiskParseFunc)(xmlXPathContextPtr ctxt, +virDomainDiskDefPtr disk); +typedef int (*virDomainXMLPrivateDataDiskFormatFunc)(virDomainDiskDefPtr disk, + virBufferPtr buf); + typedef int (*virDomainXMLPrivateDataStorageSourceParseFunc)(xmlXPathContextPtr ctxt, virStorageSourcePtr src); typedef int (*virDomainXMLPrivateDataStorageSourceFormatFunc)(virStorageSourcePtr src, @@ -2749,6 +2754,8 @@ struct _virDomainXMLPrivateDataCallbacks { /* note that private data for devices are not copied when using * virDomainDefCopy and similar functions */ virDomainXMLPrivateDataNewFuncdiskNew; +virDomainXMLPrivateDataDiskParseFunc diskParse; +virDomainXMLPrivateDataDiskFormatFunc diskFormat; virDomainXMLPrivateDataNewFuncvcpuNew; virDomainXMLPrivateDataNewFuncchrSourceNew; virDomainXMLPrivateDataNewFuncvsockNew; -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 16/55] qemu: caps: Add capability for using the blockdev infrastructure
The capability currently is not enabled so that we can add individual bits first. Signed-off-by: Peter Krempa --- src/qemu/qemu_capabilities.c | 1 + src/qemu/qemu_capabilities.h | 1 + 2 files changed, 2 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 0fb800589a..6b4c14ac50 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -507,6 +507,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, /* 315 */ "vfio-pci.display", + "blockdev", ); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 9e8ad5f5c3..452761e672 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -491,6 +491,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ /* 315 */ QEMU_CAPS_VFIO_PCI_DISPLAY, /* -device vfio-pci.display */ +QEMU_CAPS_BLOCKDEV, /* -blockdev and blockdev-add are supported */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 14/55] qemu: hotplug: Don't generate alias when detaching disk
It should be impossible to lack an alias in the domain definition. Other disk types don't generate it so remove it here as well. Signed-off-by: Peter Krempa --- src/qemu/qemu_hotplug.c | 5 - 1 file changed, 5 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1488f0a7c2..1538abf155 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4902,11 +4902,6 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, goto cleanup; } -if (!detach->info.alias) { -if (qemuAssignDeviceDiskAlias(vm->def, detach) < 0) -goto cleanup; -} - if (!async) qemuDomainMarkDeviceForRemoval(vm, &detach->info); -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/55] qemu: monitor: Allow using 'id' instead of 'device' for 'block_set_io_throttle'
The 'device' argument matches only the legacy drive alias. For blockdev we need to set the throttling for a QOM id and thus we'll need to use the 'id' field. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_monitor.c | 8 +--- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 14 ++ src/qemu/qemu_monitor_json.h | 3 ++- tests/qemumonitorjsontest.c | 2 +- 6 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5a4357d7a1..99fd3bebf5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18488,7 +18488,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, * via the JSON error code from the block_set_io_throttle call */ qemuDomainObjEnterMonitor(driver, vm); -ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, +ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, NULL, &info, supportMaxOptions, set_fields & QEMU_BLOCK_IOTUNE_SET_GROUP_NAME, supportMaxLengthOptions); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 4f0bbc147d..2902c3b246 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3444,17 +3444,19 @@ qemuMonitorGetBlockJobInfo(qemuMonitorPtr mon, int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon, - const char *device, + const char *drivealias, + const char *qomid, virDomainBlockIoTuneInfoPtr info, bool supportMaxOptions, bool supportGroupNameOption, bool supportMaxLengthOptions) { -VIR_DEBUG("device=%p, info=%p", device, info); +VIR_DEBUG("drivealias=%s, qomid=%s, info=%p", + NULLSTR(drivealias), NULLSTR(qomid), info); QEMU_CHECK_MONITOR(mon); -return qemuMonitorJSONSetBlockIoThrottle(mon, device, info, +return qemuMonitorJSONSetBlockIoThrottle(mon, drivealias, qomid, info, supportMaxOptions, supportGroupNameOption, supportMaxLengthOptions); diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index b8e3ca2ce1..16fc75819f 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -932,7 +932,8 @@ int qemuMonitorOpenGraphics(qemuMonitorPtr mon, bool skipauth); int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon, - const char *device, + const char *drivealias, + const char *qomid, virDomainBlockIoTuneInfoPtr info, bool supportMaxOptions, bool supportGroupNameOption, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 48439ccae1..3a1dfb8563 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4897,7 +4897,8 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr io_throttle, #undef GET_THROTTLE_STATS_OPTIONAL int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, - const char *device, + const char *drivealias, + const char *qomid, virDomainBlockIoTuneInfoPtr info, bool supportMaxOptions, bool supportGroupNameOption, @@ -4907,12 +4908,17 @@ int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, virJSONValuePtr cmd = NULL; virJSONValuePtr result = NULL; virJSONValuePtr args = NULL; +const char *errdev = drivealias; + +if (!errdev) +errdev = qomid; if (!(cmd = qemuMonitorJSONMakeCommand("block_set_io_throttle", NULL))) return -1; if (virJSONValueObjectCreate(&args, - "s:device", device, + "S:device", drivealias, + "S:id", qomid, "U:bps", info->total_bytes_sec, "U:bps_rd", info->read_bytes_sec, "U:bps_wr", info->write_bytes_sec, @@ -4967,10 +4973,10 @@ int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, if (virJSONValueObjectHasKey(result, "error")) { if (qemuMonitorJSONHasError(result, "DeviceNotActive")) { virReportError(VIR_ERR_OPERATION_INVALID, - _("No active operation on device: %s"), device); +
[libvirt] [PATCH 06/55] Revert "qemu: monitor: Add the 'query-nodes' argument for query-blockstats"
Turns out that 'query-nodes' is not what we want and the 'query-blockstats' command was in fact buggy. Revert the new field since it's not needed. This reverts commit 50edca1331298bfcb2622e8fe588d493aff9ab68. Signed-off-by: Peter Krempa --- src/qemu/qemu_block.c| 2 +- src/qemu/qemu_monitor.c | 8 ++-- src/qemu/qemu_monitor.h | 3 +-- src/qemu/qemu_monitor_json.c | 9 +++-- src/qemu/qemu_monitor_json.h | 3 +-- 5 files changed, 8 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 66e6301210..509b0a5210 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -358,7 +358,7 @@ qemuBlockNodeNamesDetect(virQEMUDriverPtr driver, return -1; data = qemuMonitorQueryNamedBlockNodes(qemuDomainGetMonitor(vm)); -blockstats = qemuMonitorQueryBlockstats(qemuDomainGetMonitor(vm), false); +blockstats = qemuMonitorQueryBlockstats(qemuDomainGetMonitor(vm)); if (qemuDomainObjExitMonitor(driver, vm) < 0 || !data || !blockstats) goto cleanup; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 6e0644221b..4f0bbc147d 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2243,19 +2243,15 @@ qemuMonitorGetBlockInfo(qemuMonitorPtr mon) /** * qemuMonitorQueryBlockstats: * @mon: monitor object - * @nodenames: include backing chain nodes with explicitly specified name * * Returns data from a call to 'query-blockstats'. */ virJSONValuePtr -qemuMonitorQueryBlockstats(qemuMonitorPtr mon, - bool nodenames) +qemuMonitorQueryBlockstats(qemuMonitorPtr mon) { QEMU_CHECK_MONITOR_NULL(mon); -VIR_DEBUG("nodenames: %d", nodenames); - -return qemuMonitorJSONQueryBlockstats(mon, nodenames); +return qemuMonitorJSONQueryBlockstats(mon); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 2fa8d5b51d..b8e3ca2ce1 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -563,8 +563,7 @@ int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon, int qemuMonitorBlockIOStatusToError(const char *status); virHashTablePtr qemuMonitorGetBlockInfo(qemuMonitorPtr mon); -virJSONValuePtr qemuMonitorQueryBlockstats(qemuMonitorPtr mon, - bool nodenames); +virJSONValuePtr qemuMonitorQueryBlockstats(qemuMonitorPtr mon); typedef struct _qemuBlockStats qemuBlockStats; typedef qemuBlockStats *qemuBlockStatsPtr; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 2389ebb9aa..e20851eeb2 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2362,16 +2362,13 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev, virJSONValuePtr -qemuMonitorJSONQueryBlockstats(qemuMonitorPtr mon, - bool nodenames) +qemuMonitorJSONQueryBlockstats(qemuMonitorPtr mon) { virJSONValuePtr cmd; virJSONValuePtr reply = NULL; virJSONValuePtr ret = NULL; -if (!(cmd = qemuMonitorJSONMakeCommand("query-blockstats", - "B:query-nodes", nodenames, - NULL))) +if (!(cmd = qemuMonitorJSONMakeCommand("query-blockstats", NULL))) return NULL; if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) @@ -2400,7 +2397,7 @@ qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, size_t i; virJSONValuePtr devices; -if (!(devices = qemuMonitorJSONQueryBlockstats(mon, false))) +if (!(devices = qemuMonitorJSONQueryBlockstats(mon))) return -1; for (i = 0; i < virJSONValueArraySize(devices); i++) { diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 2408ab0c5b..0458d81c0d 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -86,8 +86,7 @@ int qemuMonitorJSONSetMemoryStatsPeriod(qemuMonitorPtr mon, int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, virHashTablePtr table); -virJSONValuePtr qemuMonitorJSONQueryBlockstats(qemuMonitorPtr mon, - bool nodenames); +virJSONValuePtr qemuMonitorJSONQueryBlockstats(qemuMonitorPtr mon); int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, virHashTablePtr hash, bool backingChain); -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 17/55] qemu: process: clear QEMU_CAPS_BLOCKDEV for VMs where we can't support it
SD cards are currently passed by using -drive only which would not be compatible with using -blockdev fully. Floppies at least in the case of the i440 machine type don't have a reasonable qdev ID if -drive is not used and thus would not allow queries and other operations. Since floppy drives are obsolete anyways clear blockdev when using them. Clear QEMU_CAPS_BLOCKDEV if the VM has such devices. Signed-off-by: Peter Krempa --- src/qemu/qemu_process.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c4e33723d1..6b59dbc1c3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5884,6 +5884,16 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver, qemuProcessPrepareAllowReboot(vm); +/* clear the 'blockdev' capability for VMs which have disks that need + * -drive or which have floppies where we can't reliably get the QOM path */ +for (i = 0; i < vm->def->ndisks; i++) { +if (qemuDiskBusNeedsDriveArg(vm->def->disks[i]->bus) || +vm->def->disks[i]->bus == VIR_DOMAIN_DISK_BUS_FDC) { +virQEMUCapsClear(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); +break; +} +} + /* * Normally PCI addresses are assigned in the virDomainCreate * or virDomainDefine methods. We might still need to assign -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 00/55] qemu: Add support for -blockdev
In this version everything except blockjobs should work properly. Stats were fixed so that they are properly extracted from the new output in qemu. Additionally I've also fixed the disk resize API which I forgot last time. The blockjobs support is still missing so they either will not work or will mess up internal state. That means that the last patch still should not be pushed leaving this feature dormant until we achieve full feature parity. Peter Krempa (55): tests: qemumonitorjson: Simplify debugging of 'blockInfo' test qemu: Improve errors in qemuDomainBlockResize qemu: monitor: Remove unsupported function check for 'block_resize' qemu: monitor: Remove useless 'locked' property from struct qemuDomainDiskInfo tests: qemucapabilities: Update capability data for qemu 3.0.0 Revert "qemu: monitor: Add the 'query-nodes' argument for query-blockstats" qemu: monitor: Reuse qemuMonitorJSONQueryBlock in qemuMonitorJSONBlockIoThrottleInfo qemu: monitor: Allow using 'id' instead of 'device' for 'block_set_io_throttle' qemu: monitor: Allow using 'qdev' instead of 'device' for getting disk throttling qemu: monitor: Add 'nodename' argument for 'block_resize' tests: qemu: Drop disk from hostdev-mdev tests tests: qemuxml2argv: Fork CAPS_LATEST test cases for 'blockdev' tests: qemu: Add test data for backing chains and indexes qemu: hotplug: Don't generate alias when detaching disk util: virqemu: Simplify debugging if building QOM object with missing args qemu: caps: Add capability for using the blockdev infrastructure qemu: process: clear QEMU_CAPS_BLOCKDEV for VMs where we can't support it qemu: domain: Don't redetect backing chain when using -blockdev qemu: process: Don't detect nodenames when we support -blockdev conf: domain: Format out user provided backing chains in XML qemu: domain: Add infrastructure to generate block node names conf: Implement private data formatting and parsing for disks conf: Allow formatting and parsing of 'index' for disk source image qemu: Use proper backingIndex when reporting stats for backing chain qemu: Add field to store QDEV path of a disk in private data qemu: alias: Generate QDEV name of the block backend for disks qemu: domain: Add field for storing node name for copy-on-read qemu: proces: assign node names for user defined backing chains qemu: block: Add generator for the 'copy-on-read' blockdev driver qemu: domain: Prepare qemuDomainDiskGetBackendAlias for -blockdev qemu: command: format disk source commandline for -blockdev qemu: command: Add helper to check if disk throttling is enabled qemu: process: Setup disk io throttling for -blockdev qemu: driver: Use QOM backend name for disk IO throttling APIs qemu: driver: Prepare qemuDomainBlockResize for blockdev qemu: hotplug: Prepare for blockdev-add/blockdev-del with backing chains qemu: monitor: Add APIs for cdrom tray handling for -blockdev qemu: hotplug: Implement removable media change for -blockdev qemu: driver: Don't copy disk alias in qemuDomainBlocksStatsGather qemu: Explicitly find disks for stats totals qemu: monitor: Retrieve blockstats also by qdev and node-names qemu: monitor: Add APIs for refreshing disk capacity when using -blockdev qemu: driver: Don't pass 'virDomainDiskDefPtr' to qemuDomainGetStatsOneBlock qemu: monitor: Extract 'write-threshold' automatically for -blockdev qemu: Extract exporting of disk block statistics qemu: Extract exporting of the header for block stats qemu: Export stats relevant for the frontend separately qemu: Refactor control flow in qemuDomainGetStatsBlockExportDisk qemu: Report frontend stats only for the frontend entry qemu: Export stats relevant for the storage backend qemu: monitor: Report data also for 'qdev' entry in qemuMonitorJSONGetBlockInfo qemu: Use QOM path with query-block when using -blockdev qemu: driver: Allow using blockdev with qemuDomainBlocksStatsGather qemu: driver: Prepare qemuDomainGetStatsBlock (bulk disk stats) for -blockdev DO NOT APPLY: Enable QEMU_CAPS_BLOCKDEV if 'query-blockstats' works with -blockdev docs/formatdomain.html.in | 7 +- docs/schemas/domaincommon.rng | 19 + src/conf/domain_conf.c | 89 +++- src/conf/domain_conf.h | 7 + src/qemu/qemu_alias.c | 86 ++- src/qemu/qemu_alias.h | 3 +- src/qemu/qemu_block.c | 24 +- src/qemu/qemu_block.h | 2 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c| 94 +++- src/qemu/qemu_command.h| 3 + src/qemu/qemu_domain.c | 173 +- src/qemu
[libvirt] [PATCH 15/55] util: virqemu: Simplify debugging if building QOM object with missing args
Print the values so it's simpler to debug. Signed-off-by: Peter Krempa --- src/util/virqemu.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/util/virqemu.c b/src/util/virqemu.c index 30b8dc18d4..7ffa9f780e 100644 --- a/src/util/virqemu.c +++ b/src/util/virqemu.c @@ -248,8 +248,9 @@ virQEMUBuildObjectCommandlineFromJSONInternal(virBufferPtr buf, virJSONValuePtr props) { if (!type || !alias) { -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("missing 'type' or 'alias' field of QOM 'object'")); +virReportError(VIR_ERR_INTERNAL_ERROR, + _("missing 'type'(%s) or 'alias'(%s) field of QOM 'object'"), + NULLSTR(type), NULLSTR(alias)); return -1; } -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/55] qemu: monitor: Remove unsupported function check for 'block_resize'
QEMU supports 'block_resize' since 0.14 so we don't need to do explicit checking. Additionally the caller did not use the different value at all. Signed-off-by: Peter Krempa --- src/qemu/qemu_monitor_json.c | 7 --- 1 file changed, 7 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 2921f110a9..0695ec8d2c 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2550,8 +2550,6 @@ qemuMonitorJSONBlockStatsUpdateCapacity(qemuMonitorPtr mon, } -/* Return 0 on success, -1 on failure, or -2 if not supported. Size - * is in bytes. */ int qemuMonitorJSONBlockResize(qemuMonitorPtr mon, const char *device, unsigned long long size) @@ -2570,11 +2568,6 @@ int qemuMonitorJSONBlockResize(qemuMonitorPtr mon, if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; -if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { -ret = -2; -goto cleanup; -} - if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/55] qemu: monitor: Remove useless 'locked' property from struct qemuDomainDiskInfo
We don't use it for anything useful so it does not make much sense to extract it. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.h | 1 - src/qemu/qemu_monitor_json.c | 7 --- tests/qemumonitorjsontest.c | 1 - 3 files changed, 9 deletions(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index bff293fc0a..7b79d77257 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -438,7 +438,6 @@ struct _qemuDomainVcpuPrivate { struct qemuDomainDiskInfo { bool removable; -bool locked; bool tray; bool tray_open; bool empty; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 0695ec8d2c..2389ebb9aa 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2243,13 +2243,6 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, goto cleanup; } -if (virJSONValueObjectGetBoolean(dev, "locked", &info->locked) < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot read %s value"), - "locked"); -goto cleanup; -} - /* 'tray_open' is present only if the device has a tray */ if (virJSONValueObjectGetBoolean(dev, "tray_open", &info->tray_open) == 0) info->tray = true; diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 800c26240e..863fa440a3 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1647,7 +1647,6 @@ testQemuMonitorJSONqemuMonitorJSONGetBlockInfo(const void *data) if (VIR_ALLOC(info) < 0) goto cleanup; -info->locked = true; info->removable = true; info->tray = true; -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 02/55] qemu: Improve errors in qemuDomainBlockResize
Remove the pointless "empty path" check and use a better error message if the disk was not found. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fb0d4a8c7a..5a4357d7a1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10963,12 +10963,6 @@ qemuDomainBlockResize(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_BLOCK_RESIZE_BYTES, -1); -if (path[0] == '\0') { -virReportError(VIR_ERR_INVALID_ARG, - "%s", _("empty path")); -return -1; -} - /* We prefer operating on bytes. */ if ((flags & VIR_DOMAIN_BLOCK_RESIZE_BYTES) == 0) { if (size > ULLONG_MAX / 1024) { @@ -10996,7 +10990,7 @@ qemuDomainBlockResize(virDomainPtr dom, if (!(disk = virDomainDiskByName(vm->def, path, false))) { virReportError(VIR_ERR_INVALID_ARG, - _("invalid path: %s"), path); + _("disk '%s' was not found in the domain config"), path); goto endjob; } -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/55] tests: qemumonitorjson: Simplify debugging of 'blockInfo' test
Print the differences in case when the expected data does not match. Signed-off-by: Peter Krempa --- tests/qemumonitorjsontest.c | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 1826c4f297..800c26240e 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1585,12 +1585,29 @@ testQemuMonitorJSONqemuMonitorJSONGetVirtType(const void *data) return ret; } + +static void +testQemuMonitorJSONGetBlockInfoPrint(const struct qemuDomainDiskInfo *d) +{ +VIR_TEST_VERBOSE("removable: %d, tray: %d, tray_open: %d, empty: %d, " + "io_status: %d, nodename: '%s'\n", + d->removable, d->tray, d->tray_open, d->empty, + d->io_status, NULLSTR(d->nodename)); +} + + static int testHashEqualQemuDomainDiskInfo(const void *value1, const void *value2) { const struct qemuDomainDiskInfo *info1 = value1, *info2 = value2; +int ret; + +if ((ret = memcmp(info1, info2, sizeof(*info1))) != 0) { +testQemuMonitorJSONGetBlockInfoPrint(info1); +testQemuMonitorJSONGetBlockInfoPrint(info2); +} -return memcmp(info1, info2, sizeof(*info1)); +return ret; } static int -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/9] docs: fix iscsi-direct XML example
On Tue, Aug 07, 2018 at 03:55:20PM +0200, Pavel Hrdina wrote: The element is part of element. Signed-off-by: Pavel Hrdina --- docs/storage.html.in | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 9/9] qemu: add support for domain disk initiator IQN
Signed-off-by: Pavel Hrdina --- src/qemu/qemu_block.c | 2 ++ src/qemu/qemu_domain.c| 9 + tests/qemuxml2argvdata/disk-network-iscsi-modern.args | 8 +++- tests/qemuxml2argvdata/disk-network-iscsi-modern.xml | 10 ++ 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 66e6301210..be120b30f0 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -819,6 +819,7 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) * lun:1, * user:"username", * password-secret:"secret-alias", + * initiator-name:"iqn.2017-04.com.example:client" * } */ @@ -860,6 +861,7 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) "s:transport", "tcp", "S:user", username, "S:password-secret", objalias, + "S:initiator-name", src->initiator.iqn, NULL)); goto cleanup; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6b50e0c484..e8960105c3 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4682,6 +4682,15 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src, return -1; } +/* Use QEMU_CAPS_ISCSI_PASSWORD_SECRET as witness that iscsi 'initiator-name' + * option is available, it was introduced at the same time. */ +if (src->initiator.iqn && +!virQEMUCapsGet(qemuCaps, QEMU_CAPS_ISCSI_PASSWORD_SECRET)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("iSCSI initiator IQN not supported with this QEMU binary")); +return -1; +} + return 0; } diff --git a/tests/qemuxml2argvdata/disk-network-iscsi-modern.args b/tests/qemuxml2argvdata/disk-network-iscsi-modern.args index 0a3e0bdb8b..a06b63a940 100644 --- a/tests/qemuxml2argvdata/disk-network-iscsi-modern.args +++ b/tests/qemuxml2argvdata/disk-network-iscsi-modern.args @@ -49,4 +49,10 @@ id=virtio-disk3 \ file.target=iqn.1992-01.com.example,file.lun=0,file.transport=tcp,format=raw,\ if=none,id=drive-scsi0-0-0-0 \ -device scsi-block,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\ -drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 +drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \ +-drive file.driver=iscsi,file.portal=example.org:3260,\ +file.target=iqn.1992-01.com.example:server,file.lun=0,file.transport=tcp,\ +file.initiator-name=iqn.1992-01.com.example:client,format=raw,if=none,\ +id=drive-scsi0-0-0-1 \ +-device scsi-block,bus=scsi0.0,channel=0,scsi-id=0,lun=1,\ +drive=drive-scsi0-0-0-1,id=scsi0-0-0-1 diff --git a/tests/qemuxml2argvdata/disk-network-iscsi-modern.xml b/tests/qemuxml2argvdata/disk-network-iscsi-modern.xml index fa2a889b54..d1be9c4e7d 100644 --- a/tests/qemuxml2argvdata/disk-network-iscsi-modern.xml +++ b/tests/qemuxml2argvdata/disk-network-iscsi-modern.xml @@ -55,6 +55,16 @@ + + + + + + + + + + -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 8/9] tests: introduce qemu disk-network-iscsi-modern test cases
This uses the new -drive options instead of iSCSI URI. Signed-off-by: Pavel Hrdina --- .../disk-network-iscsi-modern.args| 52 +++ .../disk-network-iscsi-modern.xml | 65 +++ tests/qemuxml2argvtest.c | 4 ++ 3 files changed, 121 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-network-iscsi-modern.args create mode 100644 tests/qemuxml2argvdata/disk-network-iscsi-modern.xml diff --git a/tests/qemuxml2argvdata/disk-network-iscsi-modern.args b/tests/qemuxml2argvdata/disk-network-iscsi-modern.args new file mode 100644 index 00..0a3e0bdb8b --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-iscsi-modern.args @@ -0,0 +1,52 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot c \ +-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \ +-usb \ +-drive file.driver=iscsi,file.portal=example.org:6000,\ +file.target=iqn.1992-01.com.example,file.lun=0,file.transport=tcp,format=raw,\ +if=none,id=drive-virtio-disk0 \ +-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\ +id=virtio-disk0 \ +-drive file.driver=iscsi,file.portal=example.org:6000,\ +file.target=iqn.1992-01.com.example,file.lun=1,file.transport=tcp,format=raw,\ +if=none,id=drive-virtio-disk1 \ +-device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk1,\ +id=virtio-disk1 \ +-drive file.driver=iscsi,file.portal=example.org:6000,\ +file.target=iqn.1992-01.com.example:storage,file.lun=1,file.transport=tcp,\ +file.user=myname,file.password-secret=AQCVn5hO6HzFAhAAq0NCv8jtJcIcE+HOBlMQ1A,\ +format=raw,if=none,id=drive-virtio-disk2 \ +-device virtio-blk-pci,bus=pci.0,addr=0x6,drive=drive-virtio-disk2,\ +id=virtio-disk2 \ +-drive file.driver=iscsi,file.portal=example.org:6000,\ +file.target=iqn.1992-01.com.example:storage,file.lun=2,file.transport=tcp,\ +file.user=myname,file.password-secret=AQCVn5hO6HzFAhAAq0NCv8jtJcIcE+HOBlMQ1A,\ +format=raw,if=none,id=drive-virtio-disk3 \ +-device virtio-blk-pci,bus=pci.0,addr=0x7,drive=drive-virtio-disk3,\ +id=virtio-disk3 \ +-drive file.driver=iscsi,file.portal=example.org:3260,\ +file.target=iqn.1992-01.com.example,file.lun=0,file.transport=tcp,format=raw,\ +if=none,id=drive-scsi0-0-0-0 \ +-device scsi-block,bus=scsi0.0,channel=0,scsi-id=0,lun=0,\ +drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 diff --git a/tests/qemuxml2argvdata/disk-network-iscsi-modern.xml b/tests/qemuxml2argvdata/disk-network-iscsi-modern.xml new file mode 100644 index 00..fa2a889b54 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-network-iscsi-modern.xml @@ -0,0 +1,65 @@ + + QEMUGuest1 + c7a5fdbd-edaf-9455-926a-d65c16db1809 + 219136 + 219136 + 1 + +hvm + + + + destroy + restart + destroy + +/usr/bin/qemu-system-i686 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index eb53b9afd5..16000ca59d 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1057,6 +1057,10 @@ mymain(void) DO_TEST("disk-network-nbd", NONE); DO_TEST_CAPS_LATEST("disk-network-nbd"); DO_TEST("disk-network-iscsi", QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_BLOCK); +DO_TEST("disk-network-iscsi-modern", +QEMU_CAPS_VIRTIO_SCSI, +QEMU_CAPS_SCSI_BLOCK, +QEMU_CAPS_ISCSI_PASSWORD_SECRET); DO_TEST_CAPS_LATEST("disk-network-iscsi"); DO_TEST_PARSE_ERROR("disk-network-iscsi-auth-secrettype-invalid", NONE); DO_TEST_PARSE_ERROR("disk-network-iscsi-auth-wrong-secrettype", NONE); -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/9] docs: move storage initiator def into storagecommon.rng
Signed-off-by: Pavel Hrdina --- docs/schemas/storagecommon.rng | 11 +++ docs/schemas/storagepool.rng | 11 --- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng index cb4f14f52f..9d17d934de 100644 --- a/docs/schemas/storagecommon.rng +++ b/docs/schemas/storagecommon.rng @@ -39,6 +39,17 @@ + + + + + + + + + + + diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index 11ac55d06f..74f4363106 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -336,17 +336,6 @@ - - - - - - - - - - - -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/9] conf: use virStorageSourceInitiator functions
Signed-off-by: Pavel Hrdina --- src/conf/storage_conf.c | 16 +--- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index a0c92af820..50dbc937e7 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -380,7 +380,7 @@ virStoragePoolSourceClear(virStoragePoolSourcePtr source) VIR_FREE(source->dir); VIR_FREE(source->name); virStorageAdapterClear(&source->adapter); -VIR_FREE(source->initiator.iqn); +virStorageSourceInitiatorClear(&source->initiator); virStorageAuthDefFree(source->auth); VIR_FREE(source->vendor); VIR_FREE(source->product); @@ -488,7 +488,8 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, } VIR_FREE(nodeset); -source->initiator.iqn = virXPathString("string(./initiator/iqn/@name)", ctxt); + +virStorageSourceInitiatorParseXML(ctxt, &source->initiator); nsource = virXPathNodeSet("./device", ctxt, &nodeset); if (nsource < 0) @@ -950,15 +951,8 @@ virStoragePoolSourceFormat(virBufferPtr buf, if (options->flags & VIR_STORAGE_POOL_SOURCE_NAME) virBufferEscapeString(buf, "%s\n", src->name); -if ((options->flags & VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN) && -src->initiator.iqn) { -virBufferAddLit(buf, "\n"); -virBufferAdjustIndent(buf, 2); -virBufferEscapeString(buf, "\n", - src->initiator.iqn); -virBufferAdjustIndent(buf, -2); -virBufferAddLit(buf, "\n"); -} +if (options->flags & VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN) +virStorageSourceInitiatorFormatXML(&src->initiator, buf); if (options->formatToString) { const char *format = (options->formatToString)(src->format); -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 7/9] conf: introduce initiator IQN support for domain disks
Signed-off-by: Pavel Hrdina --- docs/formatdomain.html.in | 20 docs/schemas/domaincommon.rng | 3 +++ src/conf/domain_conf.c| 10 ++ src/util/virstoragefile.c | 5 + src/util/virstoragefile.h | 2 ++ 5 files changed, 40 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index b63467bd91..a7fe8b5824 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2733,6 +2733,17 @@+ + + + + + @@ -3090,6 +3101,15 @@ It's recommended to allow libvirt manage the persistent reservations. + initiator + Since libvirt 4.7.0, the +initiator element is supported for a disk +type "network" that is using a source +element with the protocol attribute "iscsi". +If present, the initiator element provides the +initiator IQN needed to access the source via mandatory +attribute name. + diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index ac04af51a1..1a786968cc 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1699,6 +1699,9 @@ + + + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d1504610a1..14112429fe 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8826,6 +8826,8 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node, virStorageSourceNetworkAssignDefaultPorts(src); +virStorageSourceInitiatorParseXML(ctxt, &src->initiator); + ret = 0; cleanup: @@ -23541,6 +23543,8 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf, virBufferEscapeString(childBuf, "\n", src->snapshot); virBufferEscapeString(childBuf, "\n", src->configFile); +virStorageSourceInitiatorFormatXML(&src->initiator, childBuf); + return 0; } @@ -30167,6 +30171,12 @@ virDomainDiskTranslateISCSIDircect(virDomainDiskDefPtr def, if (virDomainDiskAddISCSIPoolSourceHost(def, pooldef) < 0) return -1; +if (!def->src->initiator.iqn && pooldef->source.initiator.iqn && +virStorageSourceInitiatorCopy(&def->src->initiator, + &pooldef->source.initiator) < 0) { +return -1; +} + return 0; } diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index fb79ceddd0..6939d0d6c9 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2272,6 +2272,9 @@ virStorageSourceCopy(const virStorageSource *src, !(ret->pr = virStoragePRDefCopy(src->pr))) goto error; +if (virStorageSourceInitiatorCopy(&ret->initiator, &src->initiator)) +goto error; + if (backingChain && src->backingStore) { if (!(ret->backingStore = virStorageSourceCopy(src->backingStore, true))) @@ -2503,6 +2506,8 @@ virStorageSourceClear(virStorageSourcePtr def) VIR_FREE(def->tlsAlias); VIR_FREE(def->tlsCertdir); +virStorageSourceInitiatorClear(&def->initiator); + memset(def, 0, sizeof(*def)); } diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index b6013431cc..3ff6c4f900 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -261,6 +261,8 @@ struct _virStorageSource { bool encryptionInherited; virStoragePRDefPtr pr; +virStorageSourceInitiatorDef initiator; + virObjectPtr privateData; int format; /* virStorageFileFormat in domain backing chains, but -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/9] docs: fix iscsi-direct XML example
The element is part of element. Signed-off-by: Pavel Hrdina --- docs/storage.html.in | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/storage.html.in b/docs/storage.html.in index 744819d99d..e9e6ec7423 100644 --- a/docs/storage.html.in +++ b/docs/storage.html.in @@ -447,10 +447,10 @@ -- Valid pool format types -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list-
[libvirt] [PATCH 3/9] conf: rename and move virStoragePoolSourceInitiatorAttr
This structure will be reused by domain disk images as well. Signed-off-by: Pavel Hrdina --- src/conf/storage_conf.h | 7 +-- src/util/virstoragefile.h | 6 ++ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 858623783d..4d7abbbc98 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -140,11 +140,6 @@ struct _virStoragePoolSourceDeviceExtent { int type; /* virStorageFreeType */ }; -typedef struct _virStoragePoolSourceInitiatorAttr virStoragePoolSourceInitiatorAttr; -struct _virStoragePoolSourceInitiatorAttr { -char *iqn; /* Initiator IQN */ -}; - /* * Pools can be backed by one or more devices, and some * allow us to track free space on underlying devices. @@ -189,7 +184,7 @@ struct _virStoragePoolSource { char *name; /* Initiator IQN */ -virStoragePoolSourceInitiatorAttr initiator; +virStorageSourceInitiatorDef initiator; /* Authentication information */ virStorageAuthDefPtr auth; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index c2c40edf68..d9e27a4a5f 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -226,6 +226,12 @@ struct _virStoragePRDef { char *mgralias; }; +typedef struct _virStorageSourceInitiatorDef virStorageSourceInitiatorDef; +typedef virStorageSourceInitiatorDef *virStorageSourceInitiatorDefPtr; +struct _virStorageSourceInitiatorDef { +char *iqn; /* Initiator IQN */ +}; + typedef struct _virStorageDriverData virStorageDriverData; typedef virStorageDriverData *virStorageDriverDataPtr; -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/9] util: introduce virStorageSourceInitiator functions
The same code would be used for storage pools and domain disks. Signed-off-by: Pavel Hrdina --- src/util/virstoragefile.c | 34 ++ src/util/virstoragefile.h | 15 +++ 2 files changed, 49 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 56082f34e9..fb79ceddd0 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -4314,6 +4314,40 @@ virStorageSourcePrivateDataFormatRelPath(virStorageSourcePtr src, return 0; } +void +virStorageSourceInitiatorParseXML(xmlXPathContextPtr ctxt, + virStorageSourceInitiatorDefPtr initiator) +{ +initiator->iqn = virXPathString("string(./initiator/iqn/@name)", ctxt); +} + +void +virStorageSourceInitiatorFormatXML(virStorageSourceInitiatorDefPtr initiator, + virBufferPtr buf) +{ +if (!initiator->iqn) +return; + +virBufferAddLit(buf, "\n"); +virBufferAdjustIndent(buf, 2); +virBufferEscapeString(buf, "\n", initiator->iqn); +virBufferAdjustIndent(buf, -2); +virBufferAddLit(buf, "\n"); +} + +int +virStorageSourceInitiatorCopy(virStorageSourceInitiatorDefPtr dest, + const virStorageSourceInitiatorDef *src) +{ +return VIR_STRDUP(dest->iqn, src->iqn); +} + +void +virStorageSourceInitiatorClear(virStorageSourceInitiatorDefPtr initiator) +{ +VIR_FREE(initiator->iqn); +} + static bool virStorageFileIsInitialized(const virStorageSource *src) { diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index d9e27a4a5f..b6013431cc 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -492,6 +492,21 @@ int virStorageSourcePrivateDataFormatRelPath(virStorageSourcePtr src, virBufferPtr buf); +void +virStorageSourceInitiatorParseXML(xmlXPathContextPtr ctxt, + virStorageSourceInitiatorDefPtr initiator); + +void +virStorageSourceInitiatorFormatXML(virStorageSourceInitiatorDefPtr initiator, + virBufferPtr buf); + +int +virStorageSourceInitiatorCopy(virStorageSourceInitiatorDefPtr dest, + const virStorageSourceInitiatorDef *src); + +void +virStorageSourceInitiatorClear(virStorageSourceInitiatorDefPtr initiator); + int virStorageFileInit(virStorageSourcePtr src); int virStorageFileInitAs(virStorageSourcePtr src, uid_t uid, gid_t gid); -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/9] iSCSI fixes and improvements
Pavel Hrdina (9): docs: fix iscsi-direct XML example conf: properly translate iscsi-direct storage pool conf: rename and move virStoragePoolSourceInitiatorAttr util: introduce virStorageSourceInitiator functions conf: use virStorageSourceInitiator functions docs: move storage initiator def into storagecommon.rng conf: introduce initiator IQN support for domain disks tests: introduce qemu disk-network-iscsi-modern test cases qemu: add support for domain disk initiator IQN docs/formatdomain.html.in | 20 + docs/schemas/domaincommon.rng | 3 + docs/schemas/storagecommon.rng| 11 +++ docs/schemas/storagepool.rng | 11 --- docs/storage.html.in | 6 +- src/conf/domain_conf.c| 71 +- src/conf/storage_conf.c | 16 ++-- src/conf/storage_conf.h | 7 +- src/qemu/qemu_block.c | 2 + src/qemu/qemu_domain.c| 9 +++ src/util/virstoragefile.c | 39 ++ src/util/virstoragefile.h | 23 ++ .../disk-network-iscsi-modern.args| 58 ++ .../disk-network-iscsi-modern.xml | 75 +++ tests/qemuxml2argvtest.c | 4 + 15 files changed, 306 insertions(+), 49 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-network-iscsi-modern.args create mode 100644 tests/qemuxml2argvdata/disk-network-iscsi-modern.xml -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/9] conf: properly translate iscsi-direct storage pool
We cannot simply used the same code as for iscsi storage pool because the default mode is 'host' which is not possible with iscsi-direct. Signed-off-by: Pavel Hrdina --- src/conf/domain_conf.c | 61 +- 1 file changed, 43 insertions(+), 18 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7dcbe8a20b..d1504610a1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30143,6 +30143,34 @@ virDomainDiskTranslateSourcePoolAuth(virDomainDiskDefPtr def, } +static int +virDomainDiskTranslateISCSIDircect(virDomainDiskDefPtr def, + virStoragePoolDefPtr pooldef) +{ +def->src->srcpool->actualtype = VIR_STORAGE_TYPE_NETWORK; +def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; + +if (virDomainDiskTranslateSourcePoolAuth(def, + &pooldef->source) < 0) +return -1; + +/* Source pool may not fill in the secrettype field, + * so we need to do so here + */ +if (def->src->auth && !def->src->auth->secrettype) { +const char *secrettype = +virSecretUsageTypeToString(VIR_SECRET_USAGE_TYPE_ISCSI); +if (VIR_STRDUP(def->src->auth->secrettype, secrettype) < 0) +return -1; +} + +if (virDomainDiskAddISCSIPoolSourceHost(def, pooldef) < 0) +return -1; + +return 0; +} + + int virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def) { @@ -30253,6 +30281,20 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def) break; case VIR_STORAGE_POOL_ISCSI_DIRECT: +if (def->startupPolicy) { +virReportError(VIR_ERR_XML_ERROR, "%s", + _("'startupPolicy' is only valid for " + "'file' type volume")); +goto cleanup; +} + +def->src->srcpool->mode = VIR_STORAGE_SOURCE_POOL_MODE_DIRECT; + +if (virDomainDiskTranslateISCSIDircect(def, pooldef) < 0) +goto cleanup; + +break; + case VIR_STORAGE_POOL_ISCSI: if (def->startupPolicy) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -30273,24 +30315,7 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def) break; case VIR_STORAGE_SOURCE_POOL_MODE_DIRECT: - def->src->srcpool->actualtype = VIR_STORAGE_TYPE_NETWORK; - def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; - - if (virDomainDiskTranslateSourcePoolAuth(def, -&pooldef->source) < 0) - goto cleanup; - - /* Source pool may not fill in the secrettype field, -* so we need to do so here -*/ - if (def->src->auth && !def->src->auth->secrettype) { - const char *secrettype = - virSecretUsageTypeToString(VIR_SECRET_USAGE_TYPE_ISCSI); - if (VIR_STRDUP(def->src->auth->secrettype, secrettype) < 0) - goto cleanup; - } - - if (virDomainDiskAddISCSIPoolSourceHost(def, pooldef) < 0) + if (virDomainDiskTranslateISCSIDircect(def, pooldef) < 0) goto cleanup; break; } -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: ensure default machine types don't change if QEMU changes
On Tue, 2018-08-07 at 13:22 +0100, Daniel P. Berrangé wrote: [...] > +/* Historically QEMU x86 targets defaulted to 'pc' machine type but > + * in future x86_64 might switch 'q35'. Such a change is considered s/switch/switch to/ > + * an ABI break from libvirt's POV. Other QEMU targets may not declare > + * a default macine at all, causing libvirt to use the first reported s/macine/machine/ > + * machinbe in the list. s/machinbe/machine/ [...] > +[VIR_ARCH_SPARC64] = "sun4u", > +[VIR_ARCH_UNICORE32] = "puv3", > +[VIR_ARCH_X86_64] = "pc", > +[VIR_ARCH_XTENSA] = "sim", > +[VIR_ARCH_XTENSAEB] = "sim", > + No empty line here. The list looks correct as far as the architectures I'm familiar with are concerned, so I'm going to assume whatever method you used to generate it is sound and skip double-checking it :) [...] > +if (preferredMachine && > +(STREQ_NULLABLE(mach->alias, preferredMachine) || > + STREQ(mach->name, preferredMachine))) > +preferredIdx = qemuCaps->nmachineTypes - 1; Braces around the body here. [...] > -if (defIdx) > -virQEMUCapsSetDefaultMachine(qemuCaps, defIdx); > +/* > + * We'll prefer to use our own historical default machine > + * to avoid mgmt apps seeing semantics changes when QEMU > + * alters it defaults. s/it/its/ [...] > + * Our preferred pmachine might have been compiled out of s/pmachine/machine/ > + * QEMU at build time though, so we still fallback to honouring > + * QEMU's reported default in that case > + */ > +if (preferredIdx == -1) > +preferredIdx = defIdx; > +if (preferredIdx != -1) > +virQEMUCapsSetDefaultMachine(qemuCaps, preferredIdx); This is certainly an improvement compared to the current situation, so Reviewed-by: Andrea Bolognani I wonder if we shouldn't just drop the default machine type handling altogether at this point, though. I expect that downstreams who customize the list of machine types exposed by their QEMU binaries will want to patch the table above either way: for example, anyone shipping qemu-system-aarch64 in an enterprise context will definitely want to compile out integratorcp and use virt as the default instead; they might decide to introduce a default at the QEMU level (upstream QEMU doesn't have one for aarch64) but I'd expect them to also tweak libvirt's own defaults at the same time. With that in mind, I'm not sure whether leaving the default machine type handling in is not going to cause unexpected behavior rather than being helpful. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: ensure "pc" machine is always used as default if available
On Tue, 2018-08-07 at 13:19 +0100, Daniel P. Berrangé wrote: > On Mon, Aug 06, 2018 at 11:38:06AM +0200, Andrea Bolognani wrote: > > I wonder how to handle architectures where QEMU never declared a > > default machine type, such as aarch64 and riscv64, though: I think > > it would make sense to prefer the virt machine type there, but I'm > > not entirely sure that wouldn't cause any breakages either... > > Existing libvirt behaviour is that we'll pick the first reported machine > type, so we have to preserve that. Right, makes sense. I guess with aarch64/virt first and x86_64/q35 now it won't take too long for applications to figure out they should specify the machine type explicitly rather than leaving that up to libvirt :) -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/3] qemu: Support vhost-vsock-ccw
On Mon, Aug 06, 2018 at 05:41:05PM +0200, Boris Fiuczynski wrote: Support the vhost-vsock-ccw device on S390. Since v2 - instead of reusing CAPS_LATEST adding CAPS_ARCH_LATEST (Peter Krempa) Since v1 - adjusted vsock command line generation (Ján Tomko) - added CAPS_LATEST test coverage to s390 (Ján Tomko) - adjusted new vsock tests to use CAPS_LATEST Bjoern Walk (1): tests: qemuxml2argv: add CAPS_ARCH_LATEST macro Boris Fiuczynski (2): qemu: Add ccw support for vhost-vsock news: Update for vhost-vsock-ccw support docs/news.xml | 8 + src/qemu/qemu_command.c | 8 +++-- src/qemu/qemu_domain.c| 10 -- src/qemu/qemu_domain_address.c| 7 +++- .../vhost-vsock-ccw-auto.s390x-latest.args| 32 ++ .../qemuxml2argvdata/vhost-vsock-ccw-auto.xml | 25 ++ .../vhost-vsock-ccw.s390x-latest.args | 32 ++ tests/qemuxml2argvdata/vhost-vsock-ccw.xml| 32 ++ tests/qemuxml2argvtest.c | 33 ++- .../vhost-vsock-ccw-auto.xml | 32 ++ tests/qemuxml2xmloutdata/vhost-vsock-ccw.xml | 1 + tests/qemuxml2xmltest.c | 5 +++ 12 files changed, 212 insertions(+), 13 deletions(-) create mode 100644 tests/qemuxml2argvdata/vhost-vsock-ccw-auto.s390x-latest.args create mode 100644 tests/qemuxml2argvdata/vhost-vsock-ccw-auto.xml create mode 100644 tests/qemuxml2argvdata/vhost-vsock-ccw.s390x-latest.args create mode 100644 tests/qemuxml2argvdata/vhost-vsock-ccw.xml create mode 100644 tests/qemuxml2xmloutdata/vhost-vsock-ccw-auto.xml create mode 12 tests/qemuxml2xmloutdata/vhost-vsock-ccw.xml Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: ensure default machine types don't change if QEMU changes
It is increasingly likely that some distro is going to change the default "x86" machine type in QEMU from "pc" to "q35". This will certainly break existing applications which write their XML on the assumption that it is using a "pc" machine by default. For example they'll lack a IDE CDROM and get PCIe instead of PCI which changes the topology radically. Libvirt promises to isolate applications from hypervisor changes that may cause incompatibilities, so we must ensure that we always use the "pc" machine type if it is available. Only use QEMU's own reported default machine type if "pc" does not exist. This issue is not x86-only, other arches are liable t change their default machine, while some arches don't report any default at all causing libvirt to pick the first machine in the list. Thus to guarantee stability to applications, declare a preferred default machine for all architectures we currently support with QEMU. Note this change assumes there will always be a "pc" alias as long as a versioned "pc-XXX" machine type exists. If QEMU were to ship a "pc-XXX" machine type but not provide the "pc" alias, it is too hard to decide which to default so. Versioned machine types are supposed to be considered opaque strings, so we can't apply any sensible ordering ourselves and QEMU isn't reporting the list of machines in any sensible ordering itself. Signed-off-by: Daniel P. Berrangé --- src/qemu/qemu_capabilities.c | 79 ++-- 1 file changed, 76 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 0fb800589a..045e2bd489 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2233,6 +2233,61 @@ virQEMUCapsProbeQMPDevices(virQEMUCapsPtr qemuCaps, } +/* Historically QEMU x86 targets defaulted to 'pc' machine type but + * in future x86_64 might switch 'q35'. Such a change is considered + * an ABI break from libvirt's POV. Other QEMU targets may not declare + * a default macine at all, causing libvirt to use the first reported + * machinbe in the list. + * + * Here we record a preferred default machine for all arches, so + * that we're not vulnerable to changes in QEMU defaults or machine + * list ordering. + */ +static const char *preferredMachines[VIR_ARCH_LAST] = +{ +[VIR_ARCH_ALPHA] = "clipper", +[VIR_ARCH_ARMV6L] = NULL, /* No QEMU impl */ +[VIR_ARCH_ARMV7L] = "integratorcp", +[VIR_ARCH_ARMV7B] = "integratorcp", + +[VIR_ARCH_AARCH64] = "integratorcp", +[VIR_ARCH_CRIS] = "axis-dev88", +[VIR_ARCH_I686] = "pc", +[VIR_ARCH_ITANIUM] = NULL, /* doesn't exist in QEMU any more */ +[VIR_ARCH_LM32] = "lm32-evr", + +[VIR_ARCH_M68K] = "mcf5208evb", +[VIR_ARCH_MICROBLAZE] = "petalogix-s3adsp1800", +[VIR_ARCH_MICROBLAZEEL] = "petalogix-s3adsp1800", +[VIR_ARCH_MIPS] = "malta", +[VIR_ARCH_MIPSEL] = "malta", + +[VIR_ARCH_MIPS64] = "malta", +[VIR_ARCH_MIPS64EL] = "malta", +[VIR_ARCH_OR32] = "or1k-sim", +[VIR_ARCH_PARISC] = NULL, /* No QEMU impl */ +[VIR_ARCH_PARISC64] = NULL, /* No QEMU impl */ + +[VIR_ARCH_PPC] = "g3beige", +[VIR_ARCH_PPCLE] = "g3beige", +[VIR_ARCH_PPC64] = "pseries", +[VIR_ARCH_PPC64LE] = "pseries", +[VIR_ARCH_PPCEMB] = "bamboo", + +[VIR_ARCH_S390] = NULL, /* No QEMU impl*/ +[VIR_ARCH_S390X] = "s390-ccw-virtio", +[VIR_ARCH_SH4] = "shix", +[VIR_ARCH_SH4EB] = "shix", +[VIR_ARCH_SPARC] = "SS-5", + +[VIR_ARCH_SPARC64] = "sun4u", +[VIR_ARCH_UNICORE32] = "puv3", +[VIR_ARCH_X86_64] = "pc", +[VIR_ARCH_XTENSA] = "sim", +[VIR_ARCH_XTENSAEB] = "sim", + +}; + static int virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps, qemuMonitorPtr mon) @@ -2241,7 +2296,9 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps, int nmachines = 0; int ret = -1; size_t i; -size_t defIdx = 0; +ssize_t defIdx = -1; +ssize_t preferredIdx = -1; +const char *preferredMachine = preferredMachines[qemuCaps->arch]; if ((nmachines = qemuMonitorGetMachines(mon, &machines)) < 0) return -1; @@ -2263,12 +2320,28 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps, mach->maxCpus = machines[i]->maxCpus; mach->hotplugCpus = machines[i]->hotplugCpus; +if (preferredMachine && +(STREQ_NULLABLE(mach->alias, preferredMachine) || + STREQ(mach->name, preferredMachine))) +preferredIdx = qemuCaps->nmachineTypes - 1; + if (machines[i]->isDefault) defIdx = qemuCaps->nmachineTypes - 1; } -if (defIdx) -virQEMUCapsSetDefaultMachine(qemuCaps, defIdx); +/* + * We'll prefer to use our own historical default machine + * to avoid mgmt apps seeing semantics changes when QEMU + * alters it defaults. + * + * Our preferred pmachine might have been compiled out of + * QEMU at build time though, so
Re: [libvirt] [PATCH] qemu: ensure "pc" machine is always used as default if available
On Mon, Aug 06, 2018 at 11:38:06AM +0200, Andrea Bolognani wrote: > On Fri, 2018-08-03 at 13:59 +0100, Daniel P. Berrangé wrote: > > It is increasingly likely that some distro is going to change the > > default "x86" machine type in QEMU from "pc" to "q35". This will > > certainly break existing applications which write their XML on the > > assumption that its using a "pc" machine by default. For example they'll > > s/its/it's/ > > > lack a IDE CDROM and get PCI-X instad of PCI which changes the topology > > s/PCI-X instad/PCIe instead/ > > [...] > > +/* Historically QEMU defaulted to 'pc' machine type but in > > + * future might switch 'q35'. Such a change is considered > > + * an ABI break from lbivirt's POV, so we must be sure to > > s/lbivirt/libvirt/ > > > + * keep 'pc' as default machine no matter what QEMU says. > > + */ > > +if (qemuCaps->arch == VIR_ARCH_X86_64 || > > +qemuCaps->arch == VIR_ARCH_I686) > > +preferredAlias = "pc"; > > You can use ARCH_IS_X86() here. > > Since we're shielding users from changes in the default x86 > machine type, I think it would make sense to do the same for other > architectures at the same time: for example, ppc64 should prefer > pseries, s390 should prefer s390-ccw-virtio and so on. > > I wonder how to handle architectures where QEMU never declared a > default machine type, such as aarch64 and riscv64, though: I think > it would make sense to prefer the virt machine type there, but I'm > not entirely sure that wouldn't cause any breakages either... Existing libvirt behaviour is that we'll pick the first reported machine type, so we have to preserve that. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix snapshot creating with vm xml persistent
Bobo Du (1): qemu:Fix snapshot creating with vm xml persistent src/qemu/qemu_driver.c | 1 + 1 file changed, 1 insertion(+) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu:Fix snapshot creating with vm xml persistent
the vm xml will be existed when the vm is undefined after started. blockcommit interface also has the bug with above. Step1:prepare a vm,eg:test1,start it and undefined Step2: virsh snapshot-create-as test1 --disk-only --no-metadata Step3:ls /etc/libvirt/qemu/test1.xml,then it will be exist here Signed-off-by: Bobo Du newDef); ret = 0; endjob: qemuDomainObjEndJob(driver, vm); -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rpc: Initialize a worker pool for max_workers=0 as well
Semantically, there is no difference between an uninitialized worker pool and an initialized worker pool with zero workers. Let's allow the worker pool to be initialized for max_workers=0 as well then which makes the API more symmetric and simplifies code. Validity of the worker pool is delegated to virThreadPoolGetMaxWorkers instead. This patch fixes segmentation faults in virNetServerGetThreadPoolParameters and virNetServerSetThreadPoolParameters for the case when no worker pool is actually initialized (max_workers=0). Signed-off-by: Marc Hartmayer --- src/rpc/virnetserver.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index c26637ed031d..b4461b3803cf 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -205,7 +205,7 @@ virNetServerDispatchNewMessage(virNetServerClientPtr client, virObjectRef(srv); virObjectUnlock(srv); -if (srv->workers) { +if (virThreadPoolGetMaxWorkers(srv->workers) > 0) { virNetServerJobPtr job; if (VIR_ALLOC(job) < 0) @@ -367,8 +367,7 @@ virNetServerPtr virNetServerNew(const char *name, if (!(srv = virObjectLockableNew(virNetServerClass))) return NULL; -if (max_workers && -!(srv->workers = virThreadPoolNew(min_workers, max_workers, +if (!(srv->workers = virThreadPoolNew(min_workers, max_workers, priority_workers, virNetServerHandleJob, srv))) @@ -579,21 +578,18 @@ virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv) goto error; if (virJSONValueObjectAppendNumberUint(object, "min_workers", - srv->workers == NULL ? 0 : virThreadPoolGetMinWorkers(srv->workers)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot set min_workers data in JSON document")); goto error; } if (virJSONValueObjectAppendNumberUint(object, "max_workers", - srv->workers == NULL ? 0 : virThreadPoolGetMaxWorkers(srv->workers)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot set max_workers data in JSON document")); goto error; } if (virJSONValueObjectAppendNumberUint(object, "priority_workers", - srv->workers == NULL ? 0 : virThreadPoolGetPriorityWorkers(srv->workers)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Cannot set priority_workers data in JSON document")); -- 2.13.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 10/32] util: socketaddr: use VIR_AUTOPTR for aggregate types
On Fri, Aug 03, 2018 at 02:52:12PM +0200, Erik Skultety wrote: > On Sat, Jul 28, 2018 at 11:31:25PM +0530, Sukrit Bhatnagar wrote: > > By making use of GNU C's cleanup attribute handled by the > > VIR_AUTOPTR macro for declaring aggregate pointer variables, > > majority of the calls to *Free functions can be dropped, which > > in turn leads to getting rid of most of our cleanup sections. > > > > Signed-off-by: Sukrit Bhatnagar > > --- > > src/util/virsocketaddr.c | 38 -- > > 1 file changed, 16 insertions(+), 22 deletions(-) > > > > diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c > > index eee725d..1b195cd 100644 > > --- a/src/util/virsocketaddr.c > > +++ b/src/util/virsocketaddr.c > > @@ -1193,52 +1193,46 @@ virSocketAddrPTRDomain(const virSocketAddr *addr, > > unsigned int prefix, > > char **ptr) > > { > > -virBuffer buf = VIR_BUFFER_INITIALIZER; > > +VIR_AUTOPTR(virBuffer) buf = NULL; > > I'll move this one line down. > > Reviewed-by: Erik Skultety Actually, you probably noticed from patch 13, this has the obvious virBuffer leak inside, so I retract my RB here. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 08/32] util: socketaddr: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
On Sat, Jul 28, 2018 at 11:31:23PM +0530, Sukrit Bhatnagar wrote: > Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in > src/util/viralloc.h, define a new wrapper around an existing > cleanup function which will be called when a variable declared > with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant > viralloc.h include, since that has moved from the source module into > the header. > > When a variable of type virSocketAddrPtr is declared using > VIR_AUTOPTR, the function virSocketAddrFree will be run > automatically on it when it goes out of scope. > > Signed-off-by: Sukrit Bhatnagar > --- Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: ensure "pc" machine is always used as default if available
On Fri, Aug 03, 2018 at 01:05:49PM -0300, Eduardo Habkost wrote: > On Fri, Aug 03, 2018 at 01:59:47PM +0100, Daniel P. Berrangé wrote: > > It is increasingly likely that some distro is going to change the > > default "x86" machine type in QEMU from "pc" to "q35". This will > > certainly break existing applications which write their XML on the > > assumption that its using a "pc" machine by default. For example they'll > > lack a IDE CDROM and get PCI-X instad of PCI which changes the topology > > radically. > > > > Libvirt promises to isolate applications from hypervisor changes that > > may cause incompatibilities, so we must ensure that we always use the > > "pc" machine type if it is available. Only use QEMU's own reported > > default machine type if "pc" does not exist. > > > > Note this change assumes there will always be a "pc" alias as long as a > > versioned "pc-XXX" machine type exists. If QEMU were to ship a "pc-XXX" > > machine type but not provide the "pc" alias, it is too hard to decide > > which to default so. Versioned machine types are supposed to be > > considered opaque strings, so we can't apply any sensible ordering > > ourselves and QEMU isn't reporting the list of machines in any sensible > > ordering itself. > > > > Signed-off-by: Daniel P. Berrangé > > Won't this break qemuParseCommandLine() if it sees a QEMU binary > running without "-machine"? It will assume the QEMU default is > "pc" but this may be not true. If no -machine arg is present in ARGV, then the code will lookup the default machine type for the emulator binary in the capabilities record. So this should just "do the right thing" with my changes in this patch. > > @@ -2263,12 +2274,16 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr > > qemuCaps, > > mach->maxCpus = machines[i]->maxCpus; > > mach->hotplugCpus = machines[i]->hotplugCpus; > > > > +if (STREQ_NULLABLE(mach->alias, preferredAlias)) > > +preferredIdx = qemuCaps->nmachineTypes - 1; > > + > > Isn't STREQ_NULLABLE(NULL, NULL) true? You don't want to set > preferredIdx here if preferredAlias==NULL. Opps, yes. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 01/12] conf: Add definitions for 'uid' and 'fid' PCI address attributes
Add zPCI definitions in preparation of extending the PCI address with parameters uid (user-defined identifier) and fid (PCI function identifier). Signed-off-by: Yi Min Zhao Reviewed-by: Boris Fiuczynski Reviewed-by: Stefan Zimmermann Reviewed-by: Bjoern Walk Reviewed-by: Ján Tomko --- src/util/virpci.h | 10 ++ 1 file changed, 10 insertions(+) diff --git a/src/util/virpci.h b/src/util/virpci.h index 2ac87694df..b137eb01e6 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -37,12 +37,22 @@ typedef virPCIDeviceAddress *virPCIDeviceAddressPtr; typedef struct _virPCIDeviceList virPCIDeviceList; typedef virPCIDeviceList *virPCIDeviceListPtr; +typedef struct _virZPCIDeviceAddress virZPCIDeviceAddress; +typedef virZPCIDeviceAddress *virZPCIDeviceAddressPtr; +struct _virZPCIDeviceAddress { +unsigned int zpci_fid; +unsigned int zpci_uid; +bool fid_assigned; +bool uid_assigned; +}; + struct _virPCIDeviceAddress { unsigned int domain; unsigned int bus; unsigned int slot; unsigned int function; int multi; /* virTristateSwitch */ +virZPCIDeviceAddressPtr zpci; }; typedef enum { -- Yi Min -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Check sev capability pointer before using it
On Tue, Aug 07, 2018 at 09:05:09AM +0200, Peter Krempa wrote: > On Tue, Aug 07, 2018 at 09:42:05 +0800, Han Han wrote: > > https://bugzilla.redhat.com/show_bug.cgi?id=1612009 > > > > Check sev capability pointer in function qemuGetSEVInfoToParams to avoid > > null pointer dereferences. > > > > Signed-off-by: Han Han > > --- > > src/qemu/qemu_driver.c | 6 ++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > > index fb0d4a8c7a..3daaef586f 100644 > > --- a/src/qemu/qemu_driver.c > > +++ b/src/qemu/qemu_driver.c > > @@ -21452,6 +21452,12 @@ qemuGetSEVInfoToParams(virQEMUCapsPtr qemuCaps, > > > > virCheckFlags(VIR_TYPED_PARAM_STRING_OKAY, -1); > > > > +if (!sev) { > > +virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", > > + _("SEV is not supported in this guest")); > > +return -1; > > +} > > I presume the crash happens after restart of libvirtd. The real bug is > that qemuCaps don't serialize the sev data into the status XML thus the > pointer will be cleared and NULL after libvirtd restart. > > The error message reported here is then wrong since the guest/host > support SEV but the data is not available. > > The crash would not happen otherwise as the function is guarded by > checking QEMU_CAPS_SEV_GUEST. Exactly, I'm currently working on it, Peter is right, this is not the right fix. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 05/12] qemu: Auto add pci-root for s390/s390x guests
The pci-root depends on zpci capability. So autogenerate pci-root if zpci exists. Signed-off-by: Yi Min Zhao Reviewed-by: Boris Fiuczynski Reviewed-by: Stefan Zimmermann Reviewed-by: Bjoern Walk Reviewed-by: Ján Tomko --- src/qemu/qemu_domain.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index de056272e8..a84e5f06b2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3227,6 +3227,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, case VIR_ARCH_S390X: addDefaultUSB = false; addPanicDevice = true; +addPCIRoot = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI); break; case VIR_ARCH_SPARC: -- Yi Min -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 09/12] qemu: Generate and use zPCI device in QEMU command line
Add new functions to generate zPCI command string and append it to QEMU command line. And the related tests are added. Signed-off-by: Yi Min Zhao Reviewed-by: Boris Fiuczynski Reviewed-by: Stefan Zimmermann Reviewed-by: Bjoern Walk Reviewed-by: Ján Tomko --- src/qemu/qemu_command.c| 114 + src/qemu/qemu_command.h| 4 + tests/qemuxml2argvdata/disk-virtio-s390-zpci.args | 1 + .../hostdev-vfio-zpci-autogenerate.args| 26 + .../hostdev-vfio-zpci-autogenerate.xml | 18 .../hostdev-vfio-zpci-boundaries.args | 30 ++ .../hostdev-vfio-zpci-boundaries.xml | 26 + .../hostdev-vfio-zpci-multidomain-many.args| 40 .../hostdev-vfio-zpci-multidomain-many.xml | 67 tests/qemuxml2argvdata/hostdev-vfio-zpci.args | 2 + tests/qemuxml2argvtest.c | 13 +++ .../hostdev-vfio-zpci-autogenerate.xml | 30 ++ .../hostdev-vfio-zpci-boundaries.xml | 42 .../hostdev-vfio-zpci-multidomain-many.xml | 79 ++ tests/qemuxml2xmltest.c| 11 ++ 15 files changed, 503 insertions(+) create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.xml create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-boundaries.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-boundaries.xml create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-zpci-autogenerate.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-zpci-boundaries.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-zpci-multidomain-many.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d148db90fa..e46c362646 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2151,6 +2151,69 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, return NULL; } +char * +qemuBuildZPCIDevStr(virDomainDeviceInfoPtr dev) +{ +virBuffer buf = VIR_BUFFER_INITIALIZER; + +virBufferAddLit(&buf, "zpci"); +virBufferAsprintf(&buf, ",uid=%u", dev->addr.pci.zpci->zpci_uid); +virBufferAsprintf(&buf, ",fid=%u", dev->addr.pci.zpci->zpci_fid); +virBufferAsprintf(&buf, ",target=%s", dev->alias); +virBufferAsprintf(&buf, ",id=zpci%u", dev->addr.pci.zpci->zpci_uid); + +if (virBufferCheckError(&buf) < 0) { +virBufferFreeAndReset(&buf); +return NULL; +} + +return virBufferContentAndReset(&buf); +} + +int +qemuCheckDeviceIsZPCI(virDomainDeviceInfoPtr info) +{ +if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && +info->pciAddressExtFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) { +return 1; +} + +if (info->addr.pci.zpci) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support zpci devices")); +return -1; +} + +return 0; +} + +static int +qemuAppendZPCIDevStr(virCommandPtr cmd, + virDomainDeviceInfoPtr dev) +{ +char *devstr = NULL; + +virCommandAddArg(cmd, "-device"); +if (!(devstr = qemuBuildZPCIDevStr(dev))) +return -1; + +virCommandAddArg(cmd, devstr); + +VIR_FREE(devstr); +return 0; +} + +static int +qemuBuildExtensionCommandLine(virCommandPtr cmd, + virDomainDeviceInfoPtr dev) +{ +int ret; + +if ((ret = qemuCheckDeviceIsZPCI(dev)) == 1) +return qemuAppendZPCIDevStr(cmd, dev); + +return ret; +} static int qemuBuildFloppyCommandLineOptions(virCommandPtr cmd, @@ -2294,6 +2357,9 @@ qemuBuildDiskCommandLine(virCommandPtr cmd, bootindex) < 0) return -1; } else { +if (qemuBuildExtensionCommandLine(cmd, &disk->info) < 0) +return -1; + virCommandAddArg(cmd, "-device"); if (!(optstr = qemuBuildDiskDeviceStr(def, disk, bootindex, @@ -2493,6 +2559,9 @@ qemuBuildFSDevCommandLine(virCommandPtr cmd, virCommandAddArg(cmd, optstr); VIR_FREE(optstr); +if (qemuBuildExtensionCommandLine(cmd, &fs->info) < 0) +return -1; + virCommandAddArg(cmd, "-device"); if (!(optstr = qemuBuildFSDevStr(def, fs, qemuCaps))) return -1; @@ -2977,6 +3046,10 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd, goto cleanup; if (devstr) { +if (qemuBuildExtensionCommandLine(cmd, &cont->info) < 0) { +VIR_FREE(devstr); +goto cleanup; +} vi