search

virtio-win / virtio-win-guest-tools-installer

113 stars 19 forks source link

Add dependencies to the VirtioFsSvc service #30

Open jwatt opened 2 years ago

jwatt commented 2 years ago

The service has two dependencies (WinFsp.Launcher and VirtioFsDrv) as noted, for example, at:

https://virtio-fs.gitlab.io/howto-windows.html https://github.com/virtio-win/kvm-guest-drivers-windows/issues/550#issuecomment-779047192

ovirt-infra commented 2 years ago

Hello contributor, thanks for submitting a PR for this project!

I am the bot who triggers "standard-CI" builds for this project. As a security measure, I will not run automated tests on PRs that are not from white-listed contributors.

In order to allow automated tests to run, please ask one of the project maintainers to review the code and then do one of the following:

  1. Type ci test please on this PR to trigger automated tests for it.
  2. Type ci add to whitelist on this PR to trigger automated tests for it and also add you to the contributor white-list so that your future PRs will be tested automatically. ( keep in mind this list might be overwritten if the job XML is refreshed, for permanent whitelisting, please follow #3 option )
  3. If you are planning to contribute to more than one project, maybe it's better to ask them to add you to the project organization, so you'll be able to run tests for all the organization's projects.
jwatt commented 2 years ago

Thanks for taking a look at this pull request, @viktor-prutyanov.

Maybe @hammerg or @xiagao can comment since they have both previously said that WinFsp.Launcher is a dependency (hammerg's comment) (xiagao's comment).

virtiofs is built on top of WinFsp, as you'll know, and my (possibly incorrect) understanding from the discussion on some virtiofs issues is that if WinFsp isn't fully ready when VirtioFsSvc is started, then virtiofs sharing may (or may not - racey?) fail for certain tasks (such as opening files in subdirectories of the shared directory, I think). To fix that, VirtioFsSvc needs to be manually restarted after boot. So I'm guessing marking WinFsp.Launcher as a dependency isn't because it's needed to launch VirtioFsSvc, but instead it may be a convenient way to have Service Manager wait until it's safe before it starts VirtioFsSvc.

xiagao commented 2 years ago

Thanks for taking a look at this pull request, @viktor-prutyanov.

Maybe @hammerg or @xiagao can comment since they have both previously said that WinFsp.Launcher is a dependency (hammerg's comment) (xiagao's comment).

Actually I config VirtioFsSvc according to https://virtio-fs.gitlab.io/howto-windows.html . And I tried without any dependecy, virtiofs still works.

virtiofs is built on top of WinFsp, as you'll know, and my (possibly incorrect) understanding from the discussion on some virtiofs issues is that if WinFsp isn't fully ready when VirtioFsSvc is started, then virtiofs sharing may (or may not - racey?) fail for certain tasks (such as opening files in subdirectories of the shared directory, I think). To fix that, VirtioFsSvc needs to be manually restarted after boot. So I'm guessing marking WinFsp.Launcher as a dependency isn't because it's needed to launch VirtioFsSvc, but instead it may be a convenient way to have Service Manager wait until it's safe before it starts VirtioFsSvc.

viktor-prutyanov commented 1 year ago

Hi @jwatt

Do you plan to send a new version of the patch?