[qemu] ensure `dnsmasq.hosts` exists #2099

surahman · 2021-05-09T17:43:15Z

With respect to issue #1118:
Added a check on DNSMasq bootstrapping for dnsmasq.hosts file in the constructor. If absent, an empty file will be created in the data_dir.

townsend2010

Hi @surahman!

Thanks for this! I have a couple of inline niggles and I would also like to ask you to add a test under tests/qemu/test_dnsmasq_server.cpp please.

src/platform/backends/qemu/dnsmasq_server.cpp

codecov · 2021-05-10T12:27:56Z

Codecov Report

Merging #2099 (1b61b12) into main (981cf15) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2099   +/-   ##
=======================================
  Coverage   81.48%   81.48%           
=======================================
  Files         184      184           
  Lines        9456     9459    +3     
=======================================
+ Hits         7705     7708    +3     
  Misses       1751     1751

Impacted Files	Coverage Δ
src/platform/backends/qemu/dnsmasq_server.cpp	`93.82% <100.00%> (+0.23%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 981cf15...1b61b12. Read the comment docs.

Saviq · 2021-05-10T14:15:09Z

Will reopen to kick the CLA check.

Saviq · 2021-05-10T14:48:28Z

@surahman hey, would you please --reset-author on your commits to include your e-mail address? We need to connect that to your CLA signature.

surahman · 2021-05-10T16:03:49Z

@Saviq sorry about that, privacy settings on Github were complaining about email address exposure before.

Saviq · 2021-05-10T18:41:22Z

@surahman the gitignore commit still has the - email. But please remove that commit anyway :)

surahman · 2021-05-10T19:08:56Z

@Saviq I am hoping that has corrected the issue...

Saviq · 2021-05-10T19:11:09Z

@surahman it did, thanks! Now we just need you to actually go through the process of signing it: :)

https://ubuntu.com/legal/contributors/agreement

surahman · 2021-05-10T19:14:19Z

@Saviq sorry I thought I already had - I just filled it out again.

Saviq · 2021-05-10T19:16:48Z

@Saviq sorry I thought I already had - I just filled it out again.

Thanks, maybe it didn't go through our legal wheels… Now I know you did, I'll follow up our side.

Thanks a bunch, again, for your help!

townsend2010 · 2021-05-17T12:18:40Z

bors try

bors · 2021-05-17T13:02:25Z

try

Build failed:

CI (Linux, qemu)

townsend2010 · 2021-05-17T13:07:12Z

Ok, all we're missing a simple little test to ensure the dnsmasq.hosts file does indeed exist. I see the codecov reports the line being covered, but I think an explicit test would be beneficial.

surahman · 2021-05-17T16:05:02Z

I have added a quick test. Is there anything I need to do to fix the CI issue?
mkdir: cannot create directory '/etc/systemd/system': File exists

townsend2010 · 2021-05-17T16:07:46Z

Thanks!

Is there anything I need to do to fix the CI issue?

I'll for this CI run to complete and then look at the error, if there is one.

townsend2010 · 2021-05-17T16:41:09Z

Looks like CI passed fine with your added test. I'm not sure where you saw that failure before though.

townsend2010

Ok, this looks good. Thanks!

bors merge

2094: [network] Better handle network caching r=luis4a0 a=townsend2010 This better handles trying to get cached network data if the network is unreachable. Also adds unit testing to URLDownloader. 2099: [qemu] ensure `dnsmasq.hosts` exists r=townsend2010 a=surahman **_With respect to issue #1118:_** Added a check on `DNSMasq` bootstrapping for `dnsmasq.hosts` file in the constructor. If absent, an empty file will be created in the `data_dir`. Co-authored-by: Chris Townsend <christopher.townsend@canonical.com> Co-authored-by: Saad Ur Rahman <saad.ur.rahman@gmail.com>

bors · 2021-05-17T17:31:20Z

Build failed (retrying...):

Linux

2099: [qemu] ensure `dnsmasq.hosts` exists r=townsend2010 a=surahman **_With respect to issue #1118:_** Added a check on `DNSMasq` bootstrapping for `dnsmasq.hosts` file in the constructor. If absent, an empty file will be created in the `data_dir`. 2101: Fix eclipsing vms in ctor r=townsend2010 a=ricab Let exceptions that arise when creating a VM in the daemon constructor through. This avoids erasing instance data that could be recoverable and important for the user, but aborts the daemon since it can't honor its database, which would otherwise be out of sync. More robust approaches for particular causes can always added. Fixes #1658. Co-authored-by: Saad Ur Rahman <saad.ur.rahman@gmail.com> Co-authored-by: Ricardo Abreu <ricardo.abreu@canonical.com>

bors · 2021-05-17T20:23:33Z

Build failed (retrying...):

CI (Linux, lxd)

2099: [qemu] ensure `dnsmasq.hosts` exists r=townsend2010 a=surahman **_With respect to issue #1118:_** Added a check on `DNSMasq` bootstrapping for `dnsmasq.hosts` file in the constructor. If absent, an empty file will be created in the `data_dir`. Co-authored-by: Saad Ur Rahman <saad.ur.rahman@gmail.com>

bors · 2021-05-17T20:37:54Z

Build failed:

Linux

Saviq · 2021-05-17T21:28:55Z

Sorry @surahman, we're having some reliability issues with a couple tests in CI…

bors retry

2099: [qemu] ensure `dnsmasq.hosts` exists r=townsend2010 a=surahman **_With respect to issue #1118:_** Added a check on `DNSMasq` bootstrapping for `dnsmasq.hosts` file in the constructor. If absent, an empty file will be created in the `data_dir`. Co-authored-by: Saad Ur Rahman <saad.ur.rahman@gmail.com>

surahman · 2021-05-17T22:02:59Z

@Saviq no worries, it is interesting to see the workflow :)

bors · 2021-05-17T22:08:53Z

Build failed:

Windows

Saviq · 2021-05-18T08:22:33Z

bors retry

2099: [qemu] ensure `dnsmasq.hosts` exists r=townsend2010 a=surahman **_With respect to issue #1118:_** Added a check on `DNSMasq` bootstrapping for `dnsmasq.hosts` file in the constructor. If absent, an empty file will be created in the `data_dir`. 2101: Fix eclipsing vms in ctor r=townsend2010 a=ricab Let exceptions that arise when creating a VM in the daemon constructor through. This avoids erasing instance data that could be recoverable and important for the user, but aborts the daemon since it can't honor its database, which would otherwise be out of sync. More robust approaches for particular causes can always added. Fixes #1658. Co-authored-by: Saad Ur Rahman <saad.ur.rahman@gmail.com> Co-authored-by: Ricardo Abreu <ricardo.abreu@canonical.com>

bors · 2021-05-18T09:10:18Z

Build failed (retrying...):

Windows

bors · 2021-05-18T10:38:20Z

Build succeeded:

townsend2010 suggested changes May 10, 2021

View reviewed changes

src/platform/backends/qemu/dnsmasq_server.cpp Outdated Show resolved Hide resolved

src/platform/backends/qemu/dnsmasq_server.cpp Outdated Show resolved Hide resolved

Saviq changed the title ~~Issue #1118~~ [qemu] ensure dnsmasq.hosts existing May 10, 2021

Saviq marked this pull request as draft May 10, 2021 14:12

Saviq marked this pull request as ready for review May 10, 2021 14:12

Saviq closed this May 10, 2021

Saviq reopened this May 10, 2021

townsend2010 changed the title ~~[qemu] ensure dnsmasq.hosts existing~~ [qemu] ensure dnsmasq.hosts exists May 10, 2021

surahman added 3 commits May 10, 2021 14:48

[qemu] creating <dnsmasq.hosts> if not present.

3b18caf

[qemu] removing file descriptor close for <dnsmasq.hosts>.

b298bd9

[qemu] adjusting variable name for file descriptor.

a000ce2

bors bot added a commit that referenced this pull request May 17, 2021

Try #2099:

a656b26

[tests] existence test for <dnsmasq.hosts>.

1b61b12

townsend2010 approved these changes May 17, 2021

View reviewed changes

bors bot merged commit d4458ab into canonical:main May 18, 2021

townsend2010 mentioned this pull request May 18, 2021

error message during boot about dnsmasq.host #1118

Closed

Saviq linked an issue Jun 23, 2021 that may be closed by this pull request

error message during boot about dnsmasq.host #1118

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[qemu] ensure `dnsmasq.hosts` exists #2099

[qemu] ensure `dnsmasq.hosts` exists #2099

surahman commented May 9, 2021

townsend2010 left a comment

codecov bot commented May 10, 2021 •

edited

Loading

Saviq commented May 10, 2021

Saviq commented May 10, 2021

surahman commented May 10, 2021 •

edited

Loading

Saviq commented May 10, 2021

surahman commented May 10, 2021

Saviq commented May 10, 2021

surahman commented May 10, 2021

Saviq commented May 10, 2021

townsend2010 commented May 17, 2021

bors bot commented May 17, 2021

townsend2010 commented May 17, 2021

surahman commented May 17, 2021

townsend2010 commented May 17, 2021

townsend2010 commented May 17, 2021

townsend2010 left a comment

bors bot commented May 17, 2021

bors bot commented May 17, 2021

bors bot commented May 17, 2021

Saviq commented May 17, 2021

surahman commented May 17, 2021

bors bot commented May 17, 2021

Saviq commented May 18, 2021

bors bot commented May 18, 2021

bors bot commented May 18, 2021

[qemu] ensure dnsmasq.hosts exists #2099

[qemu] ensure dnsmasq.hosts exists #2099

Conversation

surahman commented May 9, 2021

townsend2010 left a comment

Choose a reason for hiding this comment

codecov bot commented May 10, 2021 • edited Loading

Codecov Report

Saviq commented May 10, 2021

Saviq commented May 10, 2021

surahman commented May 10, 2021 • edited Loading

Saviq commented May 10, 2021

surahman commented May 10, 2021

Saviq commented May 10, 2021

surahman commented May 10, 2021

Saviq commented May 10, 2021

townsend2010 commented May 17, 2021

bors bot commented May 17, 2021

try

townsend2010 commented May 17, 2021

surahman commented May 17, 2021

townsend2010 commented May 17, 2021

townsend2010 commented May 17, 2021

townsend2010 left a comment

Choose a reason for hiding this comment

bors bot commented May 17, 2021

bors bot commented May 17, 2021

bors bot commented May 17, 2021

Saviq commented May 17, 2021

surahman commented May 17, 2021

bors bot commented May 17, 2021

Saviq commented May 18, 2021

bors bot commented May 18, 2021

bors bot commented May 18, 2021

[qemu] ensure `dnsmasq.hosts` exists #2099

[qemu] ensure `dnsmasq.hosts` exists #2099

codecov bot commented May 10, 2021 •

edited

Loading

surahman commented May 10, 2021 •

edited

Loading