On Tue, Apr 21, 2015 at 19:31:22 -0400, John Ferlan wrote:
> Adding a new XML element 'iothreadids' in order to allow defining
> specific IOThread ID's rather than relying on the algorithm to assign
> IOThread ID's starting at 1 and incrementing to iothreads count.
>
> This will allow future patches to be able to add new IOThreads by
> a specific iothread_id and of course delete any exisiting IOThread.
>
> Each iothreadids element will have 'n' <iothread> children elements
> which will have attribute "id". The "id" will allow for definition
> of any "valid" (eg > 0) iothread_id value.
>
> On input, if any <iothreadids> <iothread>'s are provided, they will
> be marked so that we only print out what we read in.
>
> On input, if no <iothreadids> are provided, the PostParse code will
> self generate a list of ID's starting at 1 and going to the number
> of iothreads defined for the domain (just like the current algorithm
> numbering scheme). A future patch will rework the existing algorithm
> to make use of the iothreadids list.
>
> On output, only print out the <iothreadids> if they were read in.
>
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
> docs/formatdomain.html.in | 30 +++++++
> docs/schemas/domaincommon.rng | 12 +++
> src/conf/domain_conf.c | 200 +++++++++++++++++++++++++++++++++++++++++-
> src/conf/domain_conf.h | 18 ++++
> src/libvirt_private.syms | 4 +
> 5 files changed, 262 insertions(+), 2 deletions(-)
>
...
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 479b4c2..da1bb7e 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2114,6 +2114,31 @@ virDomainPinDefCopy(virDomainPinDefPtr *src, int npin)
> return NULL;
> }
>
> +
> +void
> +virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def)
> +{
> + if (def)
> + VIR_FREE(def);
This breaks syntax check.
> +}
> +
> +
> +static void
> +virDomainIOThreadIDDefArrayFree(virDomainIOThreadIDDefPtr *def,
> + int nids)
> +{
> + size_t i;
> +
> + if (!def)
> + return;
> +
> + for (i = 0; i < nids; i++)
> + virDomainIOThreadIDDefFree(def[i]);
> +
> + VIR_FREE(def);
> +}
> +
> +
> void
> virDomainPinDefFree(virDomainPinDefPtr def)
> {
....
> @@ -3298,6 +3325,21 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
> return -1;
> }
>
> + /* Fully populate the IOThread ID list */
> + if (def->iothreads && def->iothreads != def->niothreadids) {
> + unsigned int iothread_id = 1;
> + while (def->niothreadids != def->iothreads) {
> + if (!virDomainIOThreadIDFind(def, iothread_id)) {
> + virDomainIOThreadIDDefPtr iothrid;
> +
> + if (!(iothrid = virDomainIOThreadIDAdd(def, iothread_id)))
> + return -1;
Unfortunately, fixing the iothread list after you parse iothread pinning
in patch 4 makes a loophole where you might force arbitrary iothread ID
without using <iothreadids>.
This code will probably need to be moved after the parsing code, despite
the fact that the postparse callback is better place to do such checks.
> + iothrid->autofill = true;
> + }
> + iothread_id++;
> + }
> + }
> +
> if (virDomainDefGetMemoryInitial(def) == 0) {
> virReportError(VIR_ERR_XML_ERROR, "%s",
> _("Memory size must be specified via <memory> or in the "
The rest of this patch looks good, but I'd like to see the above part
fixed before my final ACK.
Peter
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list