Update vhost-device-console backend & move it in main workspace

[TimosAmpel]

Summary of the PR

This PR updates the current implementation of vhost-device-console and addresses
the comments noted at the PR #717.

Specifically,

• Eliminates the use of 'select' function and 'nix' dependency
• Registers all input FDs to the main worker epoll handler
• Increases the unit tests' coverage
• Move the device into the main workspace

Requirements

• All commits in this PR have Signed-Off-By trailers (with
  git commit -s), and the commit message has max 60 characters for the
  summary and max 75 characters for each description line.
• All added/changed functionality has a corresponding unit/integration
  test.
• All added/changed public-facing functionality has entries in the "Upcoming
  Release" section of CHANGELOG.md (if no such section exists, please create one).
• Any newly added unsafe code is properly documented.

[stefano-garzarella]

Summary of the PR

This PR updates the current implementation of vhost-device-console and addresses the comments noted at the PR #717.

Specifically,

* Eliminates the use of 'select' function and 'nix' dependency

Cool, thanks for that!

* Registers all input FDs to the main worker epoll handler

* Increases the unit tests' coverage

* Move the device into the main workspace

All these steps in one commit?

This list should immediately make it clear that we need at least 3/4 separate commits. Please let's try to have commits as small as possible, also to make it easier for reviewers.

[TimosAmpel]

Summary of the PR

This PR updates the current implementation of vhost-device-console and addresses the comments noted at the PR #717.
Specifically,

* Eliminates the use of 'select' function and 'nix' dependency

Cool, thanks for that!

* Registers all input FDs to the main worker epoll handler

* Increases the unit tests' coverage

* Move the device into the main workspace

All these steps in one commit?

This list should immediately make it clear that we need at least 3/4 separate commits. Please let's try to have commits as small as possible, also to make it easier for reviewers.

Thanks for the comment! I did split it into three difference commits, I believe that should be ok.

[stefano-garzarella]

Thanks for the comment! I did split it into three difference commits, I believe that should be ok.

@TimosAmpel thanks for that!

We need to adjust coverage value on both main work space and staging workspace to make CI happy.

Also I suggest better commit title/description, here some suggestions:

• patch 1

vhost-device-console: use worker's epoll for input events

Eliminate the use of select and 'nix' package. This is done by
registering the input events (stdin or tcplistener) onto the main
worker's epoll.

• patch 2

vhost-device-console: improve tests

[add a little descriptions of the new tests]

• patch 3

vhost-device-console: promote to main workspace

The current implementation seems ready to be promoted to the
main workspace since the device supports .... [add what we support, and what is still missing]

[TimosAmpel]

Hello @stefano-garzarella, thanks for your comments and sorry for my delayed answer.
In the last push I have updated the commit messages based on what you requested and
also modified the coverage values for both main and staging workspaces.

Since in the last commit vhost-device-console was move to main I reverted the coverage
value of the staging to what it was before that patch serie.

Let me know if you have more comments regarding those updates.

[stefano-garzarella]

@TimosAmpel great job with the coverage!

I tested it and I did a quick review. Just 1 comment, more curiosity, but in general LGTM, we can eventually improve the code later, but the main features are there.

Thanks for your work!

[stefano-garzarella]

@TimosAmpel I just noticed that Cargo.lock need to be updated on the main workspace and staging, so maybe you can do that in 3rd patch.

[TimosAmpel]

@TimosAmpel great job with the coverage!

I tested it and I did a quick review. Just 1 comment, more curiosity, but in general LGTM, we can eventually improve the code later, but the main features are there.

Thanks for your work!

Thanks a lot for the review too!

[TimosAmpel]

@TimosAmpel I just noticed that Cargo.lock need to be updated on the main workspace and staging, so maybe you can do that in 3rd patch.

The Cargo.lock files for both main and staging have been updated in 3rd patch (also the coverage under staging needed a bit of tuning, I see the CI is satisfied now)

[stefano-garzarella]

LGTM!

@epilys @stsquad @vireshk I did a quick review and tested with upstream Linux and QEMU, so I agree that we can promote it to the main workspace, also because the coverage is good enough!

[stefano-garzarella]

Just as a reminder, when we merge this we need to do the following steps:

• publish the first version on crates.io
• create a tag and a release in this repo
• unignore nix in dependabot and close dependabot: unignore nix #738

[TimosAmpel]

Just as a reminder, when we merge this we need to do the following steps:

• publish the first version on crates.io
• create a tag and a release in this repo
• unignore nix in dependabot and close dependabot: unignore nix #738

Very nice! Thanks for the review @stefano-garzarella, we'll do as soon as this is merged

[epilys]

@stefano-garzarella btw all of us are at an employee meeting this week, we might have time from Saturday onwards.

[stefano-garzarella]

@stefano-garzarella btw all of us are at an employee meeting this week, we might have time from Saturday onwards.

@epilys thanks for the update! Take your time, we don't need to rush ;-)

[stsquad]

Looks good from here too.

[TimosAmpel]

Looks good from here too.

Thanks for your feedback @stsquad.

[TimosAmpel]

Just as a reminder, when we merge this we need to do the following steps:

• publish the first version on crates.io
• create a tag and a release in this repo
• unignore nix in dependabot and close dependabot: unignore nix #738

Hello @garzarella and @epilys,
Since both console and CAN device have been moved on the main workspace,
do you think that we could also complete the above points? (mainly the crates.io
and the creation of the tag/release)

[epilys]

@TimosAmpel certainly, the usual workflow is to make an issue and a maintainer will assign themselves to make the release. I will make an issue myself in a moment

[TimosAmpel]

@TimosAmpel certainly, the usual workflow is to make an issue and a maintainer will assign themselves to make the release. I will make an issue myself in a moment

Thanks @epilys, I will do so the next time :)