Add support for establishing a Xen event channel from outside LibVMI

[flyn-org]

Signed-off-by: W. Michael Petullo mike@flyn.org

[libvmi-jenkins]

Can one of the admins verify this patch?

[flyn-org]

This is the first attempt at making it possible to integrate libvmi into a select/poll event loop. This implementation covers Xen 4.6/4.7 and 4.8, but I have not yet tested 4.8. I have not attempted to cover KVM because I do not have experience with KVM.

For example (the new function, used below, is vmi_event_get_fd):

static gboolean
gt_handle_vmi_event(GIOChannel *chan, GIOCondition condition, gpointer user_data)
{
status_t status;
GtLoop *loop = user_data;

    switch (condition) {
    case G_IO_IN:
            status = vmi_events_listen(loop->vmi, 0);
            if (status != VMI_SUCCESS) {
                    fprintf(stderr, "error waiting for events\n");
                    gt_running = FALSE;
            }
            break;
    default:
            fprintf(stderr, "error reading fd\n");
            break;
    }

    if (!gt_running) {
            g_main_loop_quit(loop->g_main_loop);
    }

    return gt_running;

}

int
main(int argc, char *argv[])
{
...
vmi_fd = vmi_event_get_fd(vmi);
channel_vmi = g_io_channel_unix_new(vmi_fd);
gt_loop_add_watch(channel_vmi, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL, gt_handle_vmi_event, loop);
vmi_resume_vm(loop->vmi);
g_main_loop_run(loop->g_main_loop);
...
}

[tklengyel]

Exposing the file descriptor through LibVMI is a hypervisor specific feature-creep into the LibVMI API. As such I'm not a fan of this approach.

I would suggest that if your application wants to access the file-descriptor to have it setup the events channels using the Xen API, then use the new init system in PR #435 (https://github.com/libvmi/libvmi/pull/435/files#diff-0af4637539ddf2fd76051b0ac977e009L620) to pass in that information to LibVMI. The new init system has been specifically setup to address cases where a user would want to pass extra information for the hypervisor driver, like handles and descriptors it wouldn't be able or wouldn't want to replicate.

[smaresca]

I had meant to chime in as well. I agree with Tamas..it's a sort of API leakage we hope to avoid. With that said, I would be open to other approaches; I had hoped to explore an alternative myself, but have not had the time to do so. I was considering something using a func ptr callback that could be invoked upon fd state change. Ultimately, the exposure of a file descriptor forces a library user to specialize code which is otherwise hypervisor agnostic, pulling in headers for things like IOCTLs and errno values needed to interact with the file descriptor. A use case needing access to the file descriptor is, to me, suggestive that something could be added to the xen driver instead. Steve

…

On Wed, Feb 15, 2017 at 12:44 PM, Tamas K Lengyel ***@***.***> wrote: Exposing the file descriptor through LibVMI is a hypervisor specific feature-creep into the LibVMI API. As such I'm not a fan of this approach. I would suggest that if your application wants to access the file-descriptor to have it setup the events channels using the Xen API, then use the new init system in PR #435 <#435> (https://github.com/libvmi/ libvmi#435/files#diff-0af4637539ddf2fd76051b0ac977e009L620) to pass in that information to LibVMI. The new init system has been specifically setup to address cases where a user would want to pass extra information for the hypervisor driver, like handles and descriptors it wouldn't be able or wouldn't want to replicate. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#450 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA5auPaHeeixNm2SNeuezOXCd5uXUK1Rks5rczmMgaJpZM4L3mkd> .

[tklengyel]

@smaresca: adding an extra callback interface for fd changes is also very hypervisor specific, even if we abstract away the headers/errnos. That API would be unlikely to be used for other hypervisors. So I'm not really a fan of that approach either. IMHO having the user pass in the fd for the driver during init is a much cleaner solution, using an API that was specifically made for that type of thing.

[flyn-org]

I think @tklengyel's approach would be fine. Just so I understand, this particular capability is not yet in the init_rework branch; is this correct? If so, would you be willing to add this functionality to the branch? If not, could you provide a simple example of what this would look like. (In the latter case, I must be overlooking something in the branch's code.)

[drakvuf-jenkins]

Correct, right now there is no hypervisor driver yet that takes input from the user like that. I envisioned this feature to work by defining a new struct in libvmi.h and a new init flag that specifies what type of struct is being passed in to the driver (in case there will different types). Then the driver init can cast that void* appropriately and store the info it needs. This may need some experimentation to see how it works, so let me know. The init rework is still just a PR so if we end up having to change something it would be easy to do.

…

On Feb 15, 2017 19:30, "MikePetullo" ***@***.***> wrote: I think @tklengyel <https://github.com/tklengyel>'s approach would be fine. Just so I understand, this particular capability is not yet in the init_rework branch; is this correct? If so, would you be willing to add this functionality to the branch? If not, could you provide a simple example of what this would look like. (In the latter case, I must be overlooking something in the branch's code.) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#450 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ANYFPanCI_-sV07gss8xvHWFEL2aZqvoks5rc7TEgaJpZM4L3mkd> .

[tklengyel]

(That was me before just replied via the first email that hit my inbox.)

[flyn-org]

Okay, so the struct you describe might have an "fd" field among others. Perhaps if the field was not equal to -1, then the Xen driver would not initialize Xen itself, but would instead use the file descriptor fd. We would be very happy with this, and I agree it keeps the abstraction provided by libvmi at the right level. The application could take responsibility for peering down into the Xen level before passing things along to libvmi, but libvmi would not be responsible for itself exporting any of the Xen details back out.

We are hoping to release a prototype of the system that would use this in April. Might we be able to make progress on this before then? Or, should we plan to ship a locally-modified libvmi for the time being? I do not intend to apply any pressure; I just want to be able to plan our activities leading up to the prototype.

[tklengyel]

Absolutely. The init rework PR will get merged shortly so you can treat it as semi-stable API that we will be migrating to. So you can start using it and basing your work on it. Also, feel free to open a PR for your work if you would want early feedback even before the init rework PR is merged.

[libvmi-jenkins]

Can one of the admins verify this patch?

[flyn-org]

I just pushed a prototype which explores my understanding of @tklengyel's design. I added a new, example VMI_INIT_XEN_EVTCHN flag and a vmi_init_data_t structure. A program can set fields in the vmi_init_data_t structure and pass it to vmi_init() along with flags that indicate which fields are set.

I was able to use this to permit guestrace access to the Xen event channel file descriptor.

[tklengyel]

Looks to me like it would be fine to keep this as a void* in the API. My reasoning with having this as a generic pointer was that the drivers can cast it to whatever type they need it as, and as long as its not a complex structure that needs to be passed in, there is no need for a struct definition in the public header.

[tklengyel]

This is OK to test.

[flyn-org]

I removed the vmi_init_data_t structure.

[tklengyel]

Looks good to me! One last thing, once LibVMI closes it will close this fd as well that was passed in. I think the public header should point this out so there are no surprises to anyone who later use this feature.

[flyn-org]

Just pushed a new version which adds some documentation and provides a more descriptive commit log.

[tklengyel]

Great, thanks!