xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Ian Jackson <ian.jackson@eu.citrix.com>
To: Juergen Gross <jgross@suse.com>
Cc: wei.liu2@citrix.com, xen-devel@lists.xen.org
Subject: Re: [PATCH 0/3] libxl: add framework for device types
Date: Wed, 6 Jul 2016 17:09:36 +0100	[thread overview]
Message-ID: <22397.11584.866142.479525@mariner.uk.xensource.com> (raw)
In-Reply-To: <577CFFA6.5000104@suse.com>

Juergen Gross writes ("Re: [PATCH 0/3] libxl: add framework for device types"):
> On 06/07/16 14:47, Ian Jackson wrote:
> > Juergen Gross writes ("Re: [PATCH 0/3] libxl: add framework for device types"):
> >> On 06/07/16 13:04, Ian Jackson wrote:
> >>>> +    for (i = 0; i < d_config->num_pcidevs; i++) {
> >>>> +        rc = libxl__device_pci_add(gc, domid, &d_config->pcidevs[i], 1);
> >>>> +        if (rc < 0) {
> >>>> +            LOG(ERROR, "libxl_device_pci_add failed: %d", rc);
> >>>> +            goto out;
> >>>> +        }
> >>>> +    }
> >>>> +
> >>>
> >>> And there is similar code in 3/3 for dtdevs.  Could that be lifted
> >>> away somehow ?  (You'd have to take some care about the types, sadly;
> >>> ie, I think libxl__device_pci_add might have to start to take a
> >>> void*; maybe some macros could make things typesafe?)
> >>
> >> I thought about this idea already. I think we would end up with more
> >> code which would be rather unpleasant to read. Main reason is the
> >> need for a dtdev wrapper function and the pci backend creation.
> > 
> > I'm not sure what you mean by dtdev wrapper function.
> 
> The loop for dtdev calls a xc_ function with different parameters than
> the one for pci. We'd need a libxl__device_dtdev_add() wrapper function
> to do the xc_ call.

Oh, I see.

Sorry.  I was just suggesting that the iteration over the devices, and
the error handling, could be lifted out.  Not the contents of
libxl__device_FOO_add.

And yes, we'd need libxl__device_dtdev_add.

> 
> > But if you don't think this is feasible I won't insist on it.  The
> > approach you have is already a big improvement.
> 
> Thanks.
> 
> I'm planning to add more to it (e.g. I'd like to get rid of the
> MERGE() macro in libxl_retrieve_domain_configuration() in favor of a
> device type hook in order to be able to have _all_ stuff for one type
> in one source file.

Right.  That would be brilliant.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

      reply	other threads:[~2016-07-06 16:09 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-21 14:24 [PATCH 0/3] libxl: add framework for device types Juergen Gross
2016-06-21 14:24 ` [PATCH 1/3] " Juergen Gross
2016-06-21 14:24 ` [PATCH 2/3] libxl: refactor domcreate_attach_pci() to use device type framework Juergen Gross
2016-06-21 14:24 ` [PATCH 3/3] libxl: refactor domcreate_attach_dtdev() " Juergen Gross
2016-07-04 10:26 ` [PATCH 0/3] libxl: add framework for device types Juergen Gross
2016-07-06 11:04 ` Ian Jackson
2016-07-06 12:40   ` Juergen Gross
2016-07-06 12:47     ` Ian Jackson
2016-07-06 12:55       ` Juergen Gross
2016-07-06 16:09         ` Ian Jackson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=22397.11584.866142.479525@mariner.uk.xensource.com \
    --to=ian.jackson@eu.citrix.com \
    --cc=jgross@suse.com \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).