Repository: firecracker-microvm/firecracker
File: src/vmm/src/vstate/vcpu/x86_64.rs (Rust)

Code before:
    /// * `chunk_size`: Lenght of a chunk.
    ///
    /// # Errors
    ///
    /// * When [`kvm_bindings::Msrs::new`] returns errors.
    /// * When [`kvm_ioctls::VcpuFd::get_msrs`] returns errors.
    /// * When the return value of [`kvm_ioctls::VcpuFd::get_msrs`] (the number of entries that
    ///   could be gotten) is less than expected.
    pub fn get_msr_chunk(
        &self,
        msr_index_iter: impl Iterator<Item = u32>,
        chunk_size: usize,
    ) -> Result<Msrs, KvmVcpuError> {
        let chunk_iter = msr_index_iter.take(chunk_size);

        let mut msrs = Msrs::new(chunk_size)?;
        let msr_entries = msrs.as_mut_slice();
        for (pos, msr_index) in chunk_iter.enumerate() {
            msr_entries[pos].index = msr_index;
        }

        let nmsrs = self
            .fd
            .get_msrs(&mut msrs)
            .map_err(KvmVcpuError::VcpuGetMsrs)?;
        if nmsrs != chunk_size {
            Err(KvmVcpuError::VcpuGetMsr(msrs.as_slice()[nmsrs].index))
        } else {
            Ok(msrs)
        }
    }

    /// Get MSRs for the given MSR index list.
    ///
    /// # Arguments
    ///
    /// * `msr_index_list`: List of MSR indices
    ///
    /// # Errors
    ///
    /// * When `KvmVcpu::get_msr_chunks()` returns errors.
    pub fn get_msrs(
        &self,
        msr_index_iter: impl ExactSizeIterator<Item = u32>,
    ) -> Result<HashMap<u32, u64>, KvmVcpuError> {
        let mut msrs: HashMap<u32, u64> = HashMap::new();
        self.get_msr_chunks(msr_index_iter)?
            .iter()
            .for_each(|msr_chunk| {
                msr_chunk.as_slice().iter().for_each(|msr| {
                    msrs.insert(msr.index, msr.data);

Review comment (bug):
Mh, I'm assuming `msrs.as_slice()[nmsrs]` is supposed to be the first MSR from the request that failed to be retrieved? Can we add a comment about that? I had to scratch my head a bit about why we're looking at `nmsrs`, because at first I thought we were trying to get the last MSR that _was_ successfully retrieved (in which case this would've needed to be `nmsrs - 1` and would break if `nmsrs == 0`). 

Code after:
    /// * When [`kvm_ioctls::VcpuFd::get_msrs`] returns errors.
    /// * When the return value of [`kvm_ioctls::VcpuFd::get_msrs`] (the number of entries that
    ///   could be gotten) is less than expected.
    pub fn get_msr_chunk(
        &self,
        msr_index_iter: impl Iterator<Item = u32>,
        chunk_size: usize,
    ) -> Result<Msrs, KvmVcpuError> {
        let chunk_iter = msr_index_iter.take(chunk_size);

        let mut msrs = Msrs::new(chunk_size)?;
        let msr_entries = msrs.as_mut_slice();
        for (pos, msr_index) in chunk_iter.enumerate() {
            msr_entries[pos].index = msr_index;
        }

        let nmsrs = self
            .fd
            .get_msrs(&mut msrs)
            .map_err(KvmVcpuError::VcpuGetMsrs)?;
        // GET_MSRS returns a number of successfully set msrs.
        // If number of set msrs is not equal to the length of
        // `msrs`, then the value retuned by GET_MSRS can act
        // as an index to the problematic msr.
        if nmsrs != chunk_size {
            Err(KvmVcpuError::VcpuGetMsr(msrs.as_slice()[nmsrs].index))
        } else {
            Ok(msrs)
        }
    }

    /// Get MSRs for the given MSR index list.
    ///
    /// # Arguments
    ///
    /// * `msr_index_list`: List of MSR indices
    ///
    /// # Errors
    ///
    /// * When `KvmVcpu::get_msr_chunks()` returns errors.
    pub fn get_msrs(
        &self,
        msr_index_iter: impl ExactSizeIterator<Item = u32>,
    ) -> Result<HashMap<u32, u64>, KvmVcpuError> {
        let mut msrs: HashMap<u32, u64> = HashMap::new();
        self.get_msr_chunks(msr_index_iter)?
            .iter()
            .for_each(|msr_chunk| {
                msr_chunk.as_slice().iter().for_each(|msr| {
                    msrs.insert(msr.index, msr.data);
                });