drakrun: VM unit test and refactor

[manorit2001]

References #473

[manorit2001]

@chivay I am a bit stuck in the designing part of this test. Could you please help me in that?

1. Which VM should I create? I have noticed self-test in the starting creates hvm64 VM. Should we use that or you have any different better suggestion for this.
2. Should I monkeypatch the storage disks also for that? Right now, storage.py has a lot of hardcoded name(vm-0) which will make it harder to monkeypatch but if I don't monkeypatch them, the existing installation will be broken with the test. So, I'll have to refactor it first (creating a get_vm_name function in that).
3. A different suggestion altogether ( not related to tests ) but related to VirtualMachine. I was thinking of setting the network inside that class only. I have noticed that network is a must be when starting VMs. So, it kinda makes sense to initialize it and destroy it in that class only. Would like to hear your thoughts on that though.
4. What all tests need to be done while pausing and unpausing VMs. I am not sure on this on what I can do

[chivay]

First of all, sorry for the late reply!

1. Which VM should I create? I have noticed self-test in the starting creates hvm64 VM. Should we use that or you have any different better suggestion for this.

hvm64 is just a placeholder name from taken from Xen test framework. It doesn't really fit into our abstraction of VMs which are identified by an integer. I think that spinning up vm-1 should be a sensible decision. Would it introduce any issues?

2. Should I monkeypatch the storage disks also for that? Right now, storage.py has a lot of hardcoded name(vm-0) which will make it harder to monkeypatch but if I don't monkeypatch them, the existing installation will be broken with the test. So, I'll have to refactor it first (creating a get_vm_name function in that).

Yes, not a fan of those hardcoded values. We should probably get rid of them at some point.
Wouldn't using some other instance than vm-0 for testing solve this issue? 🤔

3. A different suggestion altogether ( not related to tests ) but related to VirtualMachine. I was thinking of setting the network inside that class only. I have noticed that network is a must be when starting VMs. So, it kinda makes sense to initialize it and destroy it in that class only. Would like to hear your thoughts on that though.

Yes that's true, AFAIK the only thing that VMs really need is a bridge interface, otherwise QEMU will protest. Adding some explicit dependency between networking setup and VirtualMachine is IMO a good idea. However I'm not sure if the VM class should manage it directly.
My first thought would be to introduce another class like VMNetwork that manages the interfaces, DNS, etc. and is passed as an argument to VirtualMachine constructor.

4. What all tests need to be done while pausing and unpausing VMs. I am not sure on this on what I can do

I think that ensuring that paused bit it set/unset (with xl list) should be enough 🤔

[manorit2001]

First of all, sorry for the late reply!

Apology accepted :p

hvm64 is just a placeholder name from taken from Xen test framework. It doesn't really fit into our abstraction of VMs which are identified by an integer.

Yeah, agreed with this.

I think that spinning up vm-1 should be a sensible decision. Would it introduce any issues?

But, If we are spinning up vm-1 we are indirectly assuming that the installation worked fine with vm-0 which is kinda counterintuitive for tests I think. I would prefer some solution in which from handling the initialization of storage disks to the creation of a brand new VM which can be destroyed later. This way, the test won't have any dependency on the original installation which we have talked about in previous PRs also

Yes, not a fan of those hardcoded values. We should probably get rid of them at some point.
Wouldn't using some other instance than vm-0 for testing solve this issue?

Umm, if we will be using vm-1 then I don't think this will matter much. But, if we are going to create a new VM then, initializing the qemu backend storage ( no lvm or zfs dependencies required ) right now consist of hardcoded vm-0 path and name which will corrupt the original installation. I'll suggest adding a get_vm_name parameter in the storage backends also which will allow things to be monkey patched easily ( Similar to what I have done with VirtualMachine class right now )

Yes that's true, AFAIK the only thing that VMs really need is a bridge interface, otherwise QEMU will protest. Adding some explicit dependency between networking setup and VirtualMachine is IMO a good idea. However I'm not sure if the VM class should manage it directly.
My first thought would be to introduce another class like VMNetwork that manages the interfaces, DNS, etc. and is passed as an argument to VirtualMachine constructor.

Hmm, sounds nice. But I think if a new class must be introduced then I will separate this task in next PR.

I think that ensuring that paused bit it set/unset (with xl list) should be enough

Cool, thanks!

[manorit2001]

All resolved

[chivay]

Generally LGTM, some minor suggestions.

[manorit2001]

Done, resolved

[chivay]

@manorit2001 please resolve conflicts with current master and I think we can merge this.