libmicrovmi icon indicating copy to clipboard operation
libmicrovmi copied to clipboard

Automatic reply from events on Drop

Open Wenzel opened this issue 4 years ago • 5 comments

On feature that I would like on the API is when an Event is being Dropped from the scope, it would automatically send a default response, which would be EventReplyType::Continue.

tasks:

  • [ ] add reference to Introspectable in the Event, so the event can call the driver
  • [ ] impl Drop for Event
  • [ ] add boolean in Event so that you can't reply twice to the same event

related: https://github.com/Wenzel/libmicrovmi/pull/49/files

thoughts @kylerky, @rageagainsthepc ?

Wenzel avatar Feb 15 '20 16:02 Wenzel

The default response sent in drop does not make much sense to me. I would like more control of what I send.

Also, I wonder if this will complicate implementation because we may need to add quite a few logic in the drop function. The function may need to know whether a response by user has already been sent before it sends a default one.

Moreover, I will think twice before adding a reference to Introspectable, which may bring some troubles with the borrow checker.

kylerky avatar Feb 15 '20 23:02 kylerky

Also, I wonder if this will complicate implementation because we may need to add quite a few logic in the drop function. The function may need to know whether a response by user has already been sent before it sends a default one.

A boolean would suffice

#[repr(C)]
pub struct Event {
    pub vcpu: u16,
    pub kind: EventType,
    driver: &dyn Introspectable, // pseudo code here, this might not be Rust compatible
    reply_sent: bool,

}

impl Event {
    // this API should be called to clearly define what event reply you want
   fn reply(reply_type: ReplyEventType) -> Result<(), Box<dyn Error>> {
        self.driver.reply_event(self, reply_type)?;
       Ok(())
   }
}

impl Drop for Event {
    fn drop(&self) {
         if !self.reply_sent {
              self.reply(self, ReplyEventType::Continue);
              self.reply_sent = true;
         }
    }
}

Moreover, I will think twice before adding a reference to Introspectable, which may bring some troubles with the borrow checker.

Yes, when I tried to implement it, I have issues with the lifetimes and such, so I gave up.

Wenzel avatar Feb 16 '20 13:02 Wenzel

Yes, when I tried to implement it, I have issues with the lifetimes and such, so I gave up.

You could use reference counting instead. I know this is very C++-ish, but this would allow you to store non-owning references aka weak pointers in your event. You need to upgrade weak pointers if you want to use them and this will fail if the object does not exist anymore. Might be worth a shot. https://doc.rust-lang.org/std/rc/struct.Weak.html

rageagainsthepc avatar Feb 16 '20 14:02 rageagainsthepc

I often use weak pointers in event callbacks for the vmi implementation in my company. Helps to alleviate segfaults. ;)

rageagainsthepc avatar Feb 16 '20 14:02 rageagainsthepc

Here is an idea: Why don't we have something like an event supervisor? The only way to listen for events is to register callbacks with the event supervisor. This would allow us to shape how events have to be handled. For example we could enforce an event reply by defining the reply as the return value of the callback.

rageagainsthepc avatar Feb 22 '20 18:02 rageagainsthepc