Nitin, Amogh, Can you have a quick look? please advice if I should revert or add a missing commit.
regards, Daan On Fri, May 16, 2014 at 12:55 PM, Milamber <milam...@apache.org> wrote: > Hello, > > This commit creating an NPE issue when you add a new kvm instance from ISO. > > See: https://issues.apache.org/jira/browse/CLOUDSTACK-6671 > > Milamber > > > Le 13/05/2014 09:33, d...@apache.org a ecrit : > >> CLOUDSTACK-6358: As a part of supporting dynamic guest OS defined by user, >> removing the hard-coded dependencies. >> This patch is for KVM >> >> 1. Local testing on KVM >> 2. Successfully got up system VMs >> 3. Successfully created a CentOS VM >> 4. Snapshots are not supported for KVM >> >> Signed off by :- Nitin Mehta<nitin.me...@citrix.com> >> >> >> Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo >> Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/02bd3d06 >> Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/02bd3d06 >> Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/02bd3d06 >> >> Branch: refs/heads/4.4 >> Commit: 02bd3d0671b0cde46f8aa7892f20aa0bb0d48d1c >> Parents: 1fb358d >> Author: Amogh Vasekar <amogh.vase...@citrix.com> >> Authored: Wed May 7 15:16:55 2014 -0700 >> Committer: Daan Hoogland <d...@onecht.net> >> Committed: Tue May 13 10:33:15 2014 +0200 >> >> ---------------------------------------------------------------------- >> .../storage/dao/GuestOSHypervisorDaoImpl.java | 6 +- >> .../kvm/resource/LibvirtComputingResource.java | 156 >> ++++++++++--------- >> .../hypervisor/kvm/resource/LibvirtVMDef.java | 9 ++ >> .../hypervisor/kvm/resource/VifDriverBase.java | 4 +- >> server/src/com/cloud/hypervisor/KVMGuru.java | 14 +- >> 5 files changed, 108 insertions(+), 81 deletions(-) >> ---------------------------------------------------------------------- >> >> >> >> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/02bd3d06/engine/schema/src/com/cloud/storage/dao/GuestOSHypervisorDaoImpl.java >> ---------------------------------------------------------------------- >> diff --git >> a/engine/schema/src/com/cloud/storage/dao/GuestOSHypervisorDaoImpl.java >> b/engine/schema/src/com/cloud/storage/dao/GuestOSHypervisorDaoImpl.java >> index 3b05120..b06cdfa 100644 >> --- >> a/engine/schema/src/com/cloud/storage/dao/GuestOSHypervisorDaoImpl.java >> +++ >> b/engine/schema/src/com/cloud/storage/dao/GuestOSHypervisorDaoImpl.java >> @@ -58,9 +58,13 @@ public class GuestOSHypervisorDaoImpl extends >> GenericDaoBase<GuestOSHypervisorVO >> @Override >> public GuestOSHypervisorVO findByOsIdAndHypervisor(long guestOsId, >> String hypervisorType, String hypervisorVersion) { >> SearchCriteria<GuestOSHypervisorVO> sc = mappingSearch.create(); >> + String version = "default"; >> + if (!(hypervisorVersion == null || hypervisorVersion.isEmpty())) >> { >> + version = hypervisorVersion; >> + } >> sc.setParameters("guest_os_id", guestOsId); >> sc.setParameters("hypervisor_type", hypervisorType); >> - sc.setParameters("hypervisor_version", hypervisorVersion); >> + sc.setParameters("hypervisor_version", version); >> return findOneBy(sc); >> } >> >> >> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/02bd3d06/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java >> ---------------------------------------------------------------------- >> diff --git >> a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java >> b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java >> index 345a8ee..b8f33e5 100755 >> --- >> a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java >> +++ >> b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java >> @@ -16,12 +16,79 @@ >> // under the License. >> package com.cloud.hypervisor.kvm.resource; >> +import java.io.BufferedOutputStream; >> +import java.io.BufferedReader; >> +import java.io.File; >> +import java.io.FileNotFoundException; >> +import java.io.FileOutputStream; >> +import java.io.FileReader; >> +import java.io.IOException; >> +import java.io.InputStream; >> +import java.io.InputStreamReader; >> +import java.io.Reader; >> +import java.net.InetAddress; >> +import java.net.URI; >> +import java.net.URISyntaxException; >> +import java.net.URL; >> +import java.net.URLConnection; >> +import java.text.DateFormat; >> +import java.text.MessageFormat; >> +import java.text.SimpleDateFormat; >> +import java.util.ArrayList; >> +import java.util.Arrays; >> +import java.util.Calendar; >> +import java.util.Collections; >> +import java.util.Comparator; >> +import java.util.Date; >> +import java.util.HashMap; >> +import java.util.HashSet; >> +import java.util.List; >> +import java.util.Map; >> +import java.util.Properties; >> +import java.util.Set; >> +import java.util.UUID; >> +import java.util.concurrent.Callable; >> +import java.util.concurrent.ConcurrentHashMap; >> +import java.util.concurrent.ExecutionException; >> +import java.util.concurrent.ExecutorService; >> +import java.util.concurrent.Executors; >> +import java.util.concurrent.Future; >> +import java.util.concurrent.TimeUnit; >> +import java.util.concurrent.TimeoutException; >> +import java.util.regex.Matcher; >> +import java.util.regex.Pattern; >> + >> +import javax.ejb.Local; >> +import javax.naming.ConfigurationException; >> + >> +import org.apache.commons.io.FileUtils; >> +import org.apache.commons.io.IOUtils; >> +import org.apache.log4j.Logger; >> +import org.libvirt.Connect; >> +import org.libvirt.Domain; >> +import org.libvirt.DomainBlockStats; >> +import org.libvirt.DomainInfo; >> +import org.libvirt.DomainInterfaceStats; >> +import org.libvirt.DomainSnapshot; >> +import org.libvirt.LibvirtException; >> +import org.libvirt.NodeInfo; >> +import org.libvirt.StorageVol; >> + >> import com.ceph.rados.IoCTX; >> import com.ceph.rados.Rados; >> import com.ceph.rados.RadosException; >> import com.ceph.rbd.Rbd; >> import com.ceph.rbd.RbdException; >> import com.ceph.rbd.RbdImage; >> + >> +import org.apache.cloudstack.storage.command.StorageSubSystemCommand; >> +import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; >> +import org.apache.cloudstack.storage.to.VolumeObjectTO; >> +import org.apache.cloudstack.utils.qemu.QemuImg; >> +import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat; >> +import org.apache.cloudstack.utils.qemu.QemuImgException; >> +import org.apache.cloudstack.utils.qemu.QemuImgFile; >> + >> import com.cloud.agent.api.Answer; >> import com.cloud.agent.api.AttachIsoCommand; >> import com.cloud.agent.api.AttachVolumeAnswer; >> @@ -202,71 +269,6 @@ import com.cloud.vm.VirtualMachine; >> import com.cloud.vm.VirtualMachine.PowerState; >> import com.cloud.vm.VirtualMachine.State; >> -import org.apache.cloudstack.storage.command.StorageSubSystemCommand; >> -import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; >> -import org.apache.cloudstack.storage.to.VolumeObjectTO; >> -import org.apache.cloudstack.utils.qemu.QemuImg; >> -import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat; >> -import org.apache.cloudstack.utils.qemu.QemuImgException; >> -import org.apache.cloudstack.utils.qemu.QemuImgFile; >> -import org.apache.commons.io.FileUtils; >> -import org.apache.commons.io.IOUtils; >> -import org.apache.log4j.Logger; >> -import org.libvirt.Connect; >> -import org.libvirt.Domain; >> -import org.libvirt.DomainBlockStats; >> -import org.libvirt.DomainInfo; >> -import org.libvirt.DomainInterfaceStats; >> -import org.libvirt.DomainSnapshot; >> -import org.libvirt.LibvirtException; >> -import org.libvirt.NodeInfo; >> -import org.libvirt.StorageVol; >> - >> -import javax.ejb.Local; >> -import javax.naming.ConfigurationException; >> - >> -import java.io.BufferedOutputStream; >> -import java.io.BufferedReader; >> -import java.io.File; >> -import java.io.FileNotFoundException; >> -import java.io.FileOutputStream; >> -import java.io.FileReader; >> -import java.io.IOException; >> -import java.io.InputStream; >> -import java.io.InputStreamReader; >> -import java.io.Reader; >> -import java.net.InetAddress; >> -import java.net.URI; >> -import java.net.URISyntaxException; >> -import java.net.URL; >> -import java.net.URLConnection; >> -import java.text.DateFormat; >> -import java.text.MessageFormat; >> -import java.text.SimpleDateFormat; >> -import java.util.ArrayList; >> -import java.util.Arrays; >> -import java.util.Calendar; >> -import java.util.Collections; >> -import java.util.Comparator; >> -import java.util.Date; >> -import java.util.HashMap; >> -import java.util.HashSet; >> -import java.util.List; >> -import java.util.Map; >> -import java.util.Properties; >> -import java.util.Set; >> -import java.util.UUID; >> -import java.util.concurrent.Callable; >> -import java.util.concurrent.ConcurrentHashMap; >> -import java.util.concurrent.ExecutionException; >> -import java.util.concurrent.ExecutorService; >> -import java.util.concurrent.Executors; >> -import java.util.concurrent.Future; >> -import java.util.concurrent.TimeoutException; >> -import java.util.concurrent.TimeUnit; >> -import java.util.regex.Matcher; >> -import java.util.regex.Pattern; >> - >> /** >> * LibvirtComputingResource execute requests on the computing/routing >> host using >> * the libvirt API >> @@ -2006,7 +2008,7 @@ public class LibvirtComputingResource extends >> ServerResourceBase implements Serv >> } >> Domain vm = getDomain(conn, vmName); >> - vm.attachDevice(getVifDriver(nicTO.getType()).plug(nicTO, "Other >> PV (32-bit)").toString()); >> + vm.attachDevice(getVifDriver(nicTO.getType()).plug(nicTO, "Other >> PV").toString()); >> } >> @@ -2043,7 +2045,7 @@ public class LibvirtComputingResource extends >> ServerResourceBase implements Serv >> } >> nicnum++; >> } >> - vm.attachDevice(getVifDriver(nic.getType()).plug(nic, "Other >> PV (32-bit)").toString()); >> + vm.attachDevice(getVifDriver(nic.getType()).plug(nic, "Other >> PV").toString()); >> return new PlugNicAnswer(cmd, true, "success"); >> } catch (LibvirtException e) { >> String msg = " Plug Nic failed due to " + e.toString(); >> @@ -3641,6 +3643,7 @@ public class LibvirtComputingResource extends >> ServerResourceBase implements Serv >> uuid = getUuid(uuid); >> vm.setDomUUID(uuid); >> vm.setDomDescription(vmTO.getOs()); >> + vm.setPlatformEmulator(vmTO.getPlatformEmulator()); >> GuestDef guest = new GuestDef(); >> @@ -3924,7 +3927,7 @@ public class LibvirtComputingResource extends >> ServerResourceBase implements Serv >> volPath = physicalDisk.getPath(); >> } >> - DiskDef.diskBus diskBusType = >> getGuestDiskModel(vmSpec.getOs()); >> + DiskDef.diskBus diskBusType = >> getGuestDiskModel(vmSpec.getPlatformEmulator()); >> DiskDef disk = new DiskDef(); >> if (volume.getType() == Volume.Type.ISO) { >> if (volPath == null) { >> @@ -4004,7 +4007,7 @@ public class LibvirtComputingResource extends >> ServerResourceBase implements Serv >> } >> private void createVif(LibvirtVMDef vm, NicTO nic) throws >> InternalErrorException, LibvirtException { >> - vm.getDevices().addDevice(getVifDriver(nic.getType()).plug(nic, >> vm.getGuestOSType()).toString()); >> + vm.getDevices().addDevice(getVifDriver(nic.getType()).plug(nic, >> vm.getPlatformEmulator()).toString()); >> } >> protected CheckSshAnswer execute(CheckSshCommand cmd) { >> @@ -4938,16 +4941,15 @@ public class LibvirtComputingResource extends >> ServerResourceBase implements Serv >> } >> } >> - boolean isGuestPVEnabled(String guestOS) { >> - if (guestOS == null) { >> + boolean isGuestPVEnabled(String guestOSName) { >> + if (guestOSName == null) { >> return false; >> } >> - String guestOSName = KVMGuestOsMapper.getGuestOsName(guestOS); >> - if (guestOS.startsWith("Ubuntu") || >> guestOSName.startsWith("Fedora 13") || guestOSName.startsWith("Fedora 12") >> || guestOSName.startsWith("Fedora 11") || >> + if (guestOSName.startsWith("Ubuntu") || >> guestOSName.startsWith("Fedora 13") || guestOSName.startsWith("Fedora 12") >> || guestOSName.startsWith("Fedora 11") || >> guestOSName.startsWith("Fedora 10") || >> guestOSName.startsWith("Fedora 9") || guestOSName.startsWith("CentOS 5.3") >> || guestOSName.startsWith("CentOS 5.4") || >> - guestOSName.startsWith("CentOS 5.5") || >> guestOS.startsWith("CentOS") || guestOS.startsWith("Fedora") || >> + guestOSName.startsWith("CentOS 5.5") || >> guestOSName.startsWith("CentOS") || guestOSName.startsWith("Fedora") || >> guestOSName.startsWith("Red Hat Enterprise Linux 5.3") >> || guestOSName.startsWith("Red Hat Enterprise Linux 5.4") || >> - guestOSName.startsWith("Red Hat Enterprise Linux 5.5") || >> guestOSName.startsWith("Red Hat Enterprise Linux 6") || >> guestOS.startsWith("Debian GNU/Linux") || >> + guestOSName.startsWith("Red Hat Enterprise Linux 5.5") || >> guestOSName.startsWith("Red Hat Enterprise Linux 6") || >> guestOSName.startsWith("Debian GNU/Linux") || >> guestOSName.startsWith("FreeBSD 10") || >> guestOSName.startsWith("Other PV")) { >> return true; >> } else { >> @@ -4963,8 +4965,8 @@ public class LibvirtComputingResource extends >> ServerResourceBase implements Serv >> } >> } >> - private DiskDef.diskBus getGuestDiskModel(String guestOSType) { >> - if (isGuestPVEnabled(guestOSType)) { >> + private DiskDef.diskBus getGuestDiskModel(String platformEmulator) { >> + if (isGuestPVEnabled(platformEmulator)) { >> return DiskDef.diskBus.VIRTIO; >> } else { >> return DiskDef.diskBus.IDE; >> >> >> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/02bd3d06/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java >> ---------------------------------------------------------------------- >> diff --git >> a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java >> b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java >> index 4032305..f6c3edf 100644 >> --- >> a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java >> +++ >> b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java >> @@ -28,6 +28,7 @@ public class LibvirtVMDef { >> private String _domName; >> private String _domUUID; >> private String _desc; >> + private String _platformEmulator; >> private final Map<String, Object> components = new HashMap<String, >> Object>(); >> public static class GuestDef { >> @@ -1179,6 +1180,14 @@ public class LibvirtVMDef { >> return _desc; >> } >> + public void setPlatformEmulator(String platformEmulator) { >> + _platformEmulator = platformEmulator; >> + } >> + >> + public String getPlatformEmulator() { >> + return _platformEmulator; >> + } >> + >> public void addComp(Object comp) { >> components.put(comp.getClass().toString(), comp); >> } >> >> >> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/02bd3d06/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/VifDriverBase.java >> ---------------------------------------------------------------------- >> diff --git >> a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/VifDriverBase.java >> b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/VifDriverBase.java >> index d89d71a..53e81fd 100644 >> --- >> a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/VifDriverBase.java >> +++ >> b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/VifDriverBase.java >> @@ -47,8 +47,8 @@ public abstract class VifDriverBase implements VifDriver >> { >> @Override >> public abstract void unplug(LibvirtVMDef.InterfaceDef iface); >> - protected LibvirtVMDef.InterfaceDef.nicModel >> getGuestNicModel(String guestOSType) { >> - if (_libvirtComputingResource.isGuestPVEnabled(guestOSType)) { >> + protected LibvirtVMDef.InterfaceDef.nicModel getGuestNicModel(String >> platformEmulator) { >> + if (_libvirtComputingResource.isGuestPVEnabled(platformEmulator)) >> { >> return LibvirtVMDef.InterfaceDef.nicModel.VIRTIO; >> } else { >> return LibvirtVMDef.InterfaceDef.nicModel.E1000; >> >> >> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/02bd3d06/server/src/com/cloud/hypervisor/KVMGuru.java >> ---------------------------------------------------------------------- >> diff --git a/server/src/com/cloud/hypervisor/KVMGuru.java >> b/server/src/com/cloud/hypervisor/KVMGuru.java >> index 174b32d..4225232 100644 >> --- a/server/src/com/cloud/hypervisor/KVMGuru.java >> +++ b/server/src/com/cloud/hypervisor/KVMGuru.java >> @@ -19,19 +19,28 @@ package com.cloud.hypervisor; >> import javax.ejb.Local; >> import javax.inject.Inject; >> +import org.apache.cloudstack.storage.command.StorageSubSystemCommand; >> + >> import com.cloud.agent.api.Command; >> import com.cloud.agent.api.to.VirtualMachineTO; >> +import com.cloud.host.HostVO; >> +import com.cloud.host.dao.HostDao; >> import com.cloud.hypervisor.Hypervisor.HypervisorType; >> +import com.cloud.storage.GuestOSHypervisorVO; >> import com.cloud.storage.GuestOSVO; >> import com.cloud.storage.dao.GuestOSDao; >> +import com.cloud.storage.dao.GuestOSHypervisorDao; >> import com.cloud.utils.Pair; >> import com.cloud.vm.VirtualMachineProfile; >> -import org.apache.cloudstack.storage.command.StorageSubSystemCommand; >> @Local(value = HypervisorGuru.class) >> public class KVMGuru extends HypervisorGuruBase implements >> HypervisorGuru { >> @Inject >> GuestOSDao _guestOsDao; >> + @Inject >> + GuestOSHypervisorDao _guestOsHypervisorDao; >> + @Inject >> + HostDao _hostDao; >> @Override >> public HypervisorType getHypervisorType() { >> @@ -50,6 +59,9 @@ public class KVMGuru extends HypervisorGuruBase >> implements HypervisorGuru { >> // Determine the VM's OS description >> GuestOSVO guestOS = >> _guestOsDao.findById(vm.getVirtualMachine().getGuestOSId()); >> to.setOs(guestOS.getDisplayName()); >> + HostVO host = >> _hostDao.findById(vm.getVirtualMachine().getHostId()); >> + GuestOSHypervisorVO guestOsMapping = >> _guestOsHypervisorDao.findByOsIdAndHypervisor(guestOS.getId(), >> getHypervisorType().toString(), host.getHypervisorVersion()); >> + to.setPlatformEmulator(guestOsMapping.getGuestOsName()); >> return to; >> } >> >> > -- Daan
